You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by fi...@apache.org on 2024/10/04 20:43:35 UTC
(superset) branch master updated: fix(imports): Error when importing charts / dashboards with missing DB credentials (#30503)
This is an automated email from the ASF dual-hosted git repository.
fisjac pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 95325c4673 fix(imports): Error when importing charts / dashboards with missing DB credentials (#30503)
95325c4673 is described below
commit 95325c4673b3bad887521530710783e2553f2ac9
Author: Jack <41...@users.noreply.github.com>
AuthorDate: Fri Oct 4 15:43:29 2024 -0500
fix(imports): Error when importing charts / dashboards with missing DB credentials (#30503)
---
superset/commands/database/importers/v1/utils.py | 13 +++++++++++--
superset/db_engine_specs/bigquery.py | 2 +-
tests/integration_tests/fixtures/importexport.py | 14 ++++++++++++++
.../databases/commands/importers/v1/import_test.py | 22 ++++++++++++++++++++++
4 files changed, 48 insertions(+), 3 deletions(-)
diff --git a/superset/commands/database/importers/v1/utils.py b/superset/commands/database/importers/v1/utils.py
index bf65225d8d..c9927a033d 100644
--- a/superset/commands/database/importers/v1/utils.py
+++ b/superset/commands/database/importers/v1/utils.py
@@ -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],
@@ -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()
@@ -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
@@ -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",
diff --git a/superset/db_engine_specs/bigquery.py b/superset/db_engine_specs/bigquery.py
index 380fa297ac..11175d7957 100644
--- a/superset/db_engine_specs/bigquery.py
+++ b/superset/db_engine_specs/bigquery.py
@@ -738,7 +738,7 @@ class BigQueryEngineSpec(BaseEngineSpec): # pylint: disable=too-many-public-met
@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
diff --git a/tests/integration_tests/fixtures/importexport.py b/tests/integration_tests/fixtures/importexport.py
index 1ca3e303c6..5a778ed078 100644
--- a/tests/integration_tests/fixtures/importexport.py
+++ b/tests/integration_tests/fixtures/importexport.py
@@ -378,6 +378,20 @@ database_config: dict[str, Any] = {
"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,
diff --git a/tests/unit_tests/databases/commands/importers/v1/import_test.py b/tests/unit_tests/databases/commands/importers/v1/import_test.py
index 5a0af24f57..f101216bbf 100644
--- a/tests/unit_tests/databases/commands/importers/v1/import_test.py
+++ b/tests/unit_tests/databases/commands/importers/v1/import_test.py
@@ -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: