mirror of https://github.com/apache/superset.git
fix: Fix Big Query API for POST w/ no parameters (#14822)
* Update schemas.py * Update bigquery.py * Fix tests Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>
This commit is contained in:
parent
dfe030b835
commit
c72894725e
|
@ -233,6 +233,12 @@ class DatabaseParametersSchemaMixin:
|
||||||
values=fields.Raw(),
|
values=fields.Raw(),
|
||||||
description="DB-specific parameters for configuration",
|
description="DB-specific parameters for configuration",
|
||||||
)
|
)
|
||||||
|
configuration_method = EnumField(
|
||||||
|
ConfigurationMethod,
|
||||||
|
by_value=True,
|
||||||
|
description=configuration_method_description,
|
||||||
|
missing=ConfigurationMethod.SQLALCHEMY_FORM,
|
||||||
|
)
|
||||||
|
|
||||||
# pylint: disable=no-self-use, unused-argument
|
# pylint: disable=no-self-use, unused-argument
|
||||||
@pre_load
|
@pre_load
|
||||||
|
@ -252,7 +258,8 @@ class DatabaseParametersSchemaMixin:
|
||||||
# frontend is not passing engine inside parameters
|
# frontend is not passing engine inside parameters
|
||||||
engine = data.pop("engine", None) or parameters.pop("engine", None)
|
engine = data.pop("engine", None) or parameters.pop("engine", None)
|
||||||
|
|
||||||
if parameters:
|
configuration_method = data.get("configuration_method")
|
||||||
|
if configuration_method == ConfigurationMethod.DYNAMIC_FORM:
|
||||||
if not engine:
|
if not engine:
|
||||||
raise ValidationError(
|
raise ValidationError(
|
||||||
[
|
[
|
||||||
|
@ -269,19 +276,26 @@ class DatabaseParametersSchemaMixin:
|
||||||
)
|
)
|
||||||
engine_spec = engine_specs[engine]
|
engine_spec = engine_specs[engine]
|
||||||
|
|
||||||
if hasattr(engine_spec, "build_sqlalchemy_uri"):
|
if not hasattr(engine_spec, "build_sqlalchemy_uri"):
|
||||||
serialized_encrypted_extra = data.get("encrypted_extra", "{}")
|
raise ValidationError(
|
||||||
try:
|
[
|
||||||
encrypted_extra = json.loads(serialized_encrypted_extra)
|
_(
|
||||||
except json.decoder.JSONDecodeError:
|
'Engine spec "InvalidEngine" does not support '
|
||||||
encrypted_extra = {}
|
"being configured via individual parameters."
|
||||||
|
)
|
||||||
data[
|
]
|
||||||
"sqlalchemy_uri"
|
|
||||||
] = engine_spec.build_sqlalchemy_uri( # type: ignore
|
|
||||||
parameters, encrypted_extra
|
|
||||||
)
|
)
|
||||||
|
|
||||||
|
serialized_encrypted_extra = data.get("encrypted_extra", "{}")
|
||||||
|
try:
|
||||||
|
encrypted_extra = json.loads(serialized_encrypted_extra)
|
||||||
|
except json.decoder.JSONDecodeError:
|
||||||
|
encrypted_extra = {}
|
||||||
|
|
||||||
|
data["sqlalchemy_uri"] = engine_spec.build_sqlalchemy_uri( # type: ignore
|
||||||
|
parameters, encrypted_extra
|
||||||
|
)
|
||||||
|
|
||||||
return data
|
return data
|
||||||
|
|
||||||
|
|
||||||
|
@ -325,11 +339,6 @@ class DatabasePostSchema(Schema, DatabaseParametersSchemaMixin):
|
||||||
allow_ctas = fields.Boolean(description=allow_ctas_description)
|
allow_ctas = fields.Boolean(description=allow_ctas_description)
|
||||||
allow_cvas = fields.Boolean(description=allow_cvas_description)
|
allow_cvas = fields.Boolean(description=allow_cvas_description)
|
||||||
allow_dml = fields.Boolean(description=allow_dml_description)
|
allow_dml = fields.Boolean(description=allow_dml_description)
|
||||||
configuration_method = EnumField(
|
|
||||||
ConfigurationMethod,
|
|
||||||
by_value=True,
|
|
||||||
description=configuration_method_description,
|
|
||||||
)
|
|
||||||
force_ctas_schema = fields.String(
|
force_ctas_schema = fields.String(
|
||||||
description=force_ctas_schema_description,
|
description=force_ctas_schema_description,
|
||||||
allow_none=True,
|
allow_none=True,
|
||||||
|
@ -367,12 +376,6 @@ class DatabasePutSchema(Schema, DatabaseParametersSchemaMixin):
|
||||||
description=cache_timeout_description, allow_none=True
|
description=cache_timeout_description, allow_none=True
|
||||||
)
|
)
|
||||||
expose_in_sqllab = fields.Boolean(description=expose_in_sqllab_description)
|
expose_in_sqllab = fields.Boolean(description=expose_in_sqllab_description)
|
||||||
configuration_method = EnumField(
|
|
||||||
ConfigurationMethod,
|
|
||||||
by_value=True,
|
|
||||||
allow_none=True,
|
|
||||||
description=configuration_method_description,
|
|
||||||
)
|
|
||||||
allow_run_async = fields.Boolean(description=allow_run_async_description)
|
allow_run_async = fields.Boolean(description=allow_run_async_description)
|
||||||
allow_csv_upload = fields.Boolean(description=allow_csv_upload_description)
|
allow_csv_upload = fields.Boolean(description=allow_csv_upload_description)
|
||||||
allow_ctas = fields.Boolean(description=allow_ctas_description)
|
allow_ctas = fields.Boolean(description=allow_ctas_description)
|
||||||
|
|
|
@ -307,10 +307,12 @@ class BigQueryEngineSpec(BaseEngineSpec):
|
||||||
|
|
||||||
@classmethod
|
@classmethod
|
||||||
def build_sqlalchemy_uri(
|
def build_sqlalchemy_uri(
|
||||||
cls, _: BigQueryParametersType, encrypted_extra: Optional[Dict[str, str]] = None
|
cls, _: BigQueryParametersType, encrypted_extra: Optional[Dict[str, Any]] = None
|
||||||
) -> str:
|
) -> str:
|
||||||
if encrypted_extra:
|
if encrypted_extra:
|
||||||
project_id = encrypted_extra.get("project_id")
|
project_id = encrypted_extra.get("credentials_info", {}).get("project_id")
|
||||||
|
|
||||||
|
if project_id:
|
||||||
return f"{cls.drivername}://{project_id}"
|
return f"{cls.drivername}://{project_id}"
|
||||||
|
|
||||||
raise SupersetGenericDBErrorException(
|
raise SupersetGenericDBErrorException(
|
||||||
|
|
|
@ -510,7 +510,7 @@ class TestDatabaseApi(SupersetTestCase):
|
||||||
self.login(username="admin")
|
self.login(username="admin")
|
||||||
database_data = {
|
database_data = {
|
||||||
"database_name": "test-database-updated",
|
"database_name": "test-database-updated",
|
||||||
"configuration_method": ConfigurationMethod.DYNAMIC_FORM,
|
"configuration_method": ConfigurationMethod.SQLALCHEMY_FORM,
|
||||||
}
|
}
|
||||||
uri = f"api/v1/database/{test_database.id}"
|
uri = f"api/v1/database/{test_database.id}"
|
||||||
rv = self.client.put(uri, json=database_data)
|
rv = self.client.put(uri, json=database_data)
|
||||||
|
|
|
@ -21,6 +21,7 @@ from marshmallow import fields, Schema, ValidationError
|
||||||
|
|
||||||
from superset.databases.schemas import DatabaseParametersSchemaMixin
|
from superset.databases.schemas import DatabaseParametersSchemaMixin
|
||||||
from superset.db_engine_specs.base import BasicParametersMixin
|
from superset.db_engine_specs.base import BasicParametersMixin
|
||||||
|
from superset.models.core import ConfigurationMethod
|
||||||
|
|
||||||
|
|
||||||
class DummySchema(Schema, DatabaseParametersSchemaMixin):
|
class DummySchema(Schema, DatabaseParametersSchemaMixin):
|
||||||
|
@ -39,31 +40,34 @@ class InvalidEngine:
|
||||||
def test_database_parameters_schema_mixin(get_engine_specs):
|
def test_database_parameters_schema_mixin(get_engine_specs):
|
||||||
get_engine_specs.return_value = {"dummy_engine": DummyEngine}
|
get_engine_specs.return_value = {"dummy_engine": DummyEngine}
|
||||||
payload = {
|
payload = {
|
||||||
|
"engine": "dummy_engine",
|
||||||
|
"configuration_method": ConfigurationMethod.DYNAMIC_FORM,
|
||||||
"parameters": {
|
"parameters": {
|
||||||
"engine": "dummy_engine",
|
|
||||||
"username": "username",
|
"username": "username",
|
||||||
"password": "password",
|
"password": "password",
|
||||||
"host": "localhost",
|
"host": "localhost",
|
||||||
"port": 12345,
|
"port": 12345,
|
||||||
"database": "dbname",
|
"database": "dbname",
|
||||||
}
|
},
|
||||||
}
|
}
|
||||||
schema = DummySchema()
|
schema = DummySchema()
|
||||||
result = schema.load(payload)
|
result = schema.load(payload)
|
||||||
assert result == {
|
assert result == {
|
||||||
"sqlalchemy_uri": "dummy://username:password@localhost:12345/dbname"
|
"configuration_method": ConfigurationMethod.DYNAMIC_FORM,
|
||||||
|
"sqlalchemy_uri": "dummy://username:password@localhost:12345/dbname",
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
def test_database_parameters_schema_mixin_no_engine():
|
def test_database_parameters_schema_mixin_no_engine():
|
||||||
payload = {
|
payload = {
|
||||||
|
"configuration_method": ConfigurationMethod.DYNAMIC_FORM,
|
||||||
"parameters": {
|
"parameters": {
|
||||||
"username": "username",
|
"username": "username",
|
||||||
"password": "password",
|
"password": "password",
|
||||||
"host": "localhost",
|
"host": "localhost",
|
||||||
"port": 12345,
|
"port": 12345,
|
||||||
"dbname": "dbname",
|
"dbname": "dbname",
|
||||||
}
|
},
|
||||||
}
|
}
|
||||||
schema = DummySchema()
|
schema = DummySchema()
|
||||||
try:
|
try:
|
||||||
|
@ -80,14 +84,15 @@ def test_database_parameters_schema_mixin_no_engine():
|
||||||
def test_database_parameters_schema_mixin_invalid_engine(get_engine_specs):
|
def test_database_parameters_schema_mixin_invalid_engine(get_engine_specs):
|
||||||
get_engine_specs.return_value = {}
|
get_engine_specs.return_value = {}
|
||||||
payload = {
|
payload = {
|
||||||
|
"engine": "dummy_engine",
|
||||||
|
"configuration_method": ConfigurationMethod.DYNAMIC_FORM,
|
||||||
"parameters": {
|
"parameters": {
|
||||||
"engine": "dummy_engine",
|
|
||||||
"username": "username",
|
"username": "username",
|
||||||
"password": "password",
|
"password": "password",
|
||||||
"host": "localhost",
|
"host": "localhost",
|
||||||
"port": 12345,
|
"port": 12345,
|
||||||
"dbname": "dbname",
|
"dbname": "dbname",
|
||||||
}
|
},
|
||||||
}
|
}
|
||||||
schema = DummySchema()
|
schema = DummySchema()
|
||||||
try:
|
try:
|
||||||
|
@ -102,14 +107,15 @@ def test_database_parameters_schema_mixin_invalid_engine(get_engine_specs):
|
||||||
def test_database_parameters_schema_no_mixin(get_engine_specs):
|
def test_database_parameters_schema_no_mixin(get_engine_specs):
|
||||||
get_engine_specs.return_value = {"invalid_engine": InvalidEngine}
|
get_engine_specs.return_value = {"invalid_engine": InvalidEngine}
|
||||||
payload = {
|
payload = {
|
||||||
|
"engine": "invalid_engine",
|
||||||
|
"configuration_method": ConfigurationMethod.DYNAMIC_FORM,
|
||||||
"parameters": {
|
"parameters": {
|
||||||
"engine": "invalid_engine",
|
|
||||||
"username": "username",
|
"username": "username",
|
||||||
"password": "password",
|
"password": "password",
|
||||||
"host": "localhost",
|
"host": "localhost",
|
||||||
"port": 12345,
|
"port": 12345,
|
||||||
"database": "dbname",
|
"database": "dbname",
|
||||||
}
|
},
|
||||||
}
|
}
|
||||||
schema = DummySchema()
|
schema = DummySchema()
|
||||||
try:
|
try:
|
||||||
|
|
Loading…
Reference in New Issue