Skip to content

Commit

Permalink
fix(imports): Error when importing charts / dashboards with missing D…
Browse files Browse the repository at this point in the history
…B credentials (apache#30503)
  • Loading branch information
fisjac authored Oct 4, 2024
1 parent 68c9a81 commit 95325c4
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 3 deletions.
13 changes: 11 additions & 2 deletions superset/commands/database/importers/v1/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,21 @@
# specific language governing permissions and limitations
# under the License.

import logging
from typing import Any

from superset import app, db, security_manager
from superset.commands.exceptions import ImportFailedError
from superset.databases.ssh_tunnel.models import SSHTunnel
from superset.databases.utils import make_url_safe
from superset.db_engine_specs.exceptions import SupersetDBAPIConnectionError
from superset.exceptions import SupersetSecurityException
from superset.models.core import Database
from superset.security.analytics_db_safety import check_sqlalchemy_uri
from superset.utils import json

logger = logging.getLogger(__name__)


def import_database(
config: dict[str, Any],
Expand Down Expand Up @@ -64,7 +68,7 @@ def import_database(
# Before it gets removed in import_from_dict
ssh_tunnel_config = config.pop("ssh_tunnel", None)

database = Database.import_from_dict(config, recursive=False)
database: Database = Database.import_from_dict(config, recursive=False)
if database.id is None:
db.session.flush()

Expand All @@ -75,7 +79,11 @@ def import_database(
ssh_tunnel = None

# TODO (betodealmeida): we should use the `CreateDatabaseCommand` for imports
add_permissions(database, ssh_tunnel)

try:
add_permissions(database, ssh_tunnel)
except SupersetDBAPIConnectionError as ex:
logger.warning(ex.message)

return database

Expand All @@ -89,6 +97,7 @@ def add_permissions(database: Database, ssh_tunnel: SSHTunnel) -> None:
cache=False,
ssh_tunnel=ssh_tunnel,
)

for catalog in catalogs:
security_manager.add_permission_view_menu(
"catalog_access",
Expand Down
2 changes: 1 addition & 1 deletion superset/db_engine_specs/bigquery.py
Original file line number Diff line number Diff line change
Expand Up @@ -738,7 +738,7 @@ def _get_fields(cls, cols: list[ResultSetColumnType]) -> list[Any]:
@classmethod
def parse_error_exception(cls, exception: Exception) -> Exception:
try:
return Exception(str(exception).splitlines()[0].strip())
return type(exception)(str(exception).splitlines()[0].strip())
except Exception: # pylint: disable=broad-except
# If for some reason we get an exception, for example, no new line
# We will return the original exception
Expand Down
14 changes: 14 additions & 0 deletions tests/integration_tests/fixtures/importexport.py
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,20 @@
"uuid": "b8a1ccd3-779d-4ab7-8ad8-9ab119d7fe89",
"version": "1.0.0",
}
database_config_no_creds: dict[str, Any] = {
"allow_csv_upload": False,
"allow_ctas": False,
"allow_cvas": False,
"allow_dml": False,
"allow_run_async": False,
"cache_timeout": None,
"database_name": "imported_database_no_creds",
"expose_in_sqllab": True,
"extra": {},
"sqlalchemy_uri": "bigquery://test-db/",
"uuid": "2ff17edc-f3fa-4609-a5ac-b484281225bc",
"version": "1.0.0",
}

database_with_ssh_tunnel_config_private_key: dict[str, Any] = {
"allow_csv_upload": True,
Expand Down
22 changes: 22 additions & 0 deletions tests/unit_tests/databases/commands/importers/v1/import_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,28 @@ def test_import_database(mocker: MockerFixture, session: Session) -> None:
assert database.allow_dml is False


def test_import_database_no_creds(mocker: MockerFixture, session: Session) -> None:
"""
Test importing a database.
"""
from superset import security_manager
from superset.commands.database.importers.v1.utils import import_database
from superset.models.core import Database
from tests.integration_tests.fixtures.importexport import database_config_no_creds

mocker.patch.object(security_manager, "can_access", return_value=True)

engine = db.session.get_bind()
Database.metadata.create_all(engine) # pylint: disable=no-member

config = copy.deepcopy(database_config_no_creds)
database = import_database(config)
assert database.database_name == "imported_database_no_creds"
assert database.sqlalchemy_uri == "bigquery://test-db/"
assert database.extra == "{}"
assert database.uuid == "2ff17edc-f3fa-4609-a5ac-b484281225bc"


def test_import_database_sqlite_invalid(
mocker: MockerFixture, session: Session
) -> None:
Expand Down

0 comments on commit 95325c4

Please sign in to comment.