You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by di...@apache.org on 2024/02/21 10:13:23 UTC
(superset) 01/10: Remove tunnel via database update
This is an automated email from the ASF dual-hosted git repository.
diegopucci pushed a commit to branch diego/ch78628/fix-disabled-ssh-toggle
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 8e9b13dd0461be8589fece8c091d5aa06c3f87a1
Author: geido <di...@gmail.com>
AuthorDate: Fri Feb 16 16:10:11 2024 +0200
Remove tunnel via database update
---
.../src/features/databases/DatabaseModal/index.tsx | 33 ++++--------
superset-frontend/src/features/databases/types.ts | 6 +--
superset/commands/database/update.py | 58 +++++++++++++++-------
3 files changed, 52 insertions(+), 45 deletions(-)
diff --git a/superset-frontend/src/features/databases/DatabaseModal/index.tsx b/superset-frontend/src/features/databases/DatabaseModal/index.tsx
index 98e7cab415..46b090f465 100644
--- a/superset-frontend/src/features/databases/DatabaseModal/index.tsx
+++ b/superset-frontend/src/features/databases/DatabaseModal/index.tsx
@@ -596,7 +596,9 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
const SSHTunnelSwitchComponent =
extensionsRegistry.get('ssh_tunnel.form.switch') ?? SSHTunnelSwitch;
- const [useSSHTunneling, setUseSSHTunneling] = useState<boolean>(false);
+ const [useSSHTunneling, setUseSSHTunneling] = useState<boolean | undefined>(
+ undefined,
+ );
let dbConfigExtraExtension = extensionsRegistry.get(
'databaseconnection.extraOption',
@@ -724,7 +726,7 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
setSSHTunnelPrivateKeys({});
setSSHTunnelPrivateKeyPasswords({});
setConfirmedOverwrite(false);
- setUseSSHTunneling(false);
+ setUseSSHTunneling(undefined);
onHide();
};
@@ -850,27 +852,10 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
setLoading(true);
- // tunnel config is kept until db is saved for UX convenience (toggling on and off)
// strictly checking for false as an indication that the toggle got unchecked
- if (isSSHTunneling && dbToUpdate?.parameters?.ssh === false) {
- if (db?.id && dbFetched?.ssh_tunnel) {
- // the db and ssh tunnel exist, should be removed
- try {
- await SupersetClient.delete({
- endpoint: `/api/v1/database/${db.id}/ssh_tunnel/`,
- });
- } catch (e) {
- addDangerToast(
- t('There was an error removing the SSH tunnel configuration'),
- );
- setLoading(false);
- console.error(e);
- return;
- }
- }
- handleClearSSHTunnelConfig();
- // remove ssh tunnel from payload
- dbToUpdate.ssh_tunnel = undefined;
+ if (isSSHTunneling && useSSHTunneling === false) {
+ // remove ssh tunnel
+ dbToUpdate.ssh_tunnel = null;
}
if (db?.id) {
@@ -1325,8 +1310,8 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps> = ({
}, [sshPrivateKeyPasswordNeeded]);
useEffect(() => {
- if (isSSHTunneling) {
- setUseSSHTunneling(!!db?.parameters?.ssh);
+ if (isSSHTunneling && db?.parameters?.ssh !== undefined) {
+ setUseSSHTunneling(db?.parameters?.ssh);
handleClearValidationErrors();
}
}, [db?.parameters?.ssh]);
diff --git a/superset-frontend/src/features/databases/types.ts b/superset-frontend/src/features/databases/types.ts
index dbbd661890..2f481d8315 100644
--- a/superset-frontend/src/features/databases/types.ts
+++ b/superset-frontend/src/features/databases/types.ts
@@ -112,7 +112,7 @@ export type DatabaseObject = {
};
// SSH Tunnel information
- ssh_tunnel?: SSHTunnelObject;
+ ssh_tunnel?: SSHTunnelObject | null;
};
export type DatabaseForm = {
@@ -236,8 +236,8 @@ export interface ExtraJson {
version?: string;
}
-type ParametersChangeValueType = HTMLInputElement & {
- value: string | boolean;
+type ParametersChangeValueType = Partial<Omit<HTMLInputElement, 'value'>> & {
+ value?: string | boolean;
};
type ParametersChangeType<T = ParametersChangeValueType> =
diff --git a/superset/commands/database/update.py b/superset/commands/database/update.py
index edc0ba1b98..b891c8f157 100644
--- a/superset/commands/database/update.py
+++ b/superset/commands/database/update.py
@@ -30,8 +30,10 @@ from superset.commands.database.exceptions import (
DatabaseUpdateFailedError,
)
from superset.commands.database.ssh_tunnel.create import CreateSSHTunnelCommand
+from superset.commands.database.ssh_tunnel.delete import DeleteSSHTunnelCommand
from superset.commands.database.ssh_tunnel.exceptions import (
SSHTunnelCreateFailedError,
+ SSHTunnelDeleteFailedError,
SSHTunnelingNotEnabledError,
SSHTunnelInvalidError,
SSHTunnelUpdateFailedError,
@@ -70,32 +72,52 @@ class UpdateDatabaseCommand(BaseCommand):
database = DatabaseDAO.update(self._model, self._properties, commit=False)
database.set_sqlalchemy_uri(database.sqlalchemy_uri)
- if ssh_tunnel_properties := self._properties.get("ssh_tunnel"):
+ existing_ssh_tunnel_model = DatabaseDAO.get_ssh_tunnel(database.id)
+
+ if "ssh_tunnel" in self._properties:
if not is_feature_enabled("SSH_TUNNELING"):
db.session.rollback()
raise SSHTunnelingNotEnabledError()
- existing_ssh_tunnel_model = DatabaseDAO.get_ssh_tunnel(database.id)
- if existing_ssh_tunnel_model is None:
- # We couldn't found an existing tunnel so we need to create one
- try:
- CreateSSHTunnelCommand(database, ssh_tunnel_properties).run()
- except (SSHTunnelInvalidError, SSHTunnelCreateFailedError) as ex:
- # So we can show the original message
- raise ex
- except Exception as ex:
- raise DatabaseUpdateFailedError() from ex
- else:
- # We found an existing tunnel so we need to update it
+
+ if not self._properties.get("ssh_tunnel") and existing_ssh_tunnel_model:
+ # We need to remove the existing tunnel
try:
- UpdateSSHTunnelCommand(
- existing_ssh_tunnel_model.id, ssh_tunnel_properties
- ).run()
- except (SSHTunnelInvalidError, SSHTunnelUpdateFailedError) as ex:
- # So we can show the original message
+ DeleteSSHTunnelCommand(existing_ssh_tunnel_model.id).run()
+ except SSHTunnelDeleteFailedError as ex:
raise ex
except Exception as ex:
raise DatabaseUpdateFailedError() from ex
+ if ssh_tunnel_properties := self._properties.get("ssh_tunnel"):
+ if existing_ssh_tunnel_model is None:
+ # We couldn't found an existing tunnel so we need to create one
+ try:
+ CreateSSHTunnelCommand(
+ database, ssh_tunnel_properties
+ ).run()
+ except (
+ SSHTunnelInvalidError,
+ SSHTunnelCreateFailedError,
+ ) as ex:
+ # So we can show the original message
+ raise ex
+ except Exception as ex:
+ raise DatabaseUpdateFailedError() from ex
+ else:
+ # We found an existing tunnel so we need to update it
+ try:
+ UpdateSSHTunnelCommand(
+ existing_ssh_tunnel_model.id, ssh_tunnel_properties
+ ).run()
+ except (
+ SSHTunnelInvalidError,
+ SSHTunnelUpdateFailedError,
+ ) as ex:
+ # So we can show the original message
+ raise ex
+ except Exception as ex:
+ raise DatabaseUpdateFailedError() from ex
+
# adding a new database we always want to force refresh schema list
# TODO Improve this simplistic implementation for catching DB conn fails
try: