From 9c5de2b555e612ef73fff69d7ff032594f1e83c6 Mon Sep 17 00:00:00 2001 From: geido Date: Thu, 15 Feb 2024 19:45:11 +0200 Subject: [PATCH 1/6] Fix DELETE ssh_tunnel --- superset/databases/api.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/superset/databases/api.py b/superset/databases/api.py index 62fb72943749f..f251b45880769 100644 --- a/superset/databases/api.py +++ b/superset/databases/api.py @@ -1483,11 +1483,17 @@ def delete_ssh_tunnel(self, pk: int) -> Response: 500: $ref: '#/components/responses/500' """ - try: - DeleteSSHTunnelCommand(pk).run() - return self.response(200, message="OK") - except SSHTunnelNotFoundError: + + database = DatabaseDAO.find_by_id(pk) + if not database: return self.response_404() + try: + existing_ssh_tunnel_model = database.ssh_tunnels + if existing_ssh_tunnel_model: + DeleteSSHTunnelCommand(existing_ssh_tunnel_model.id).run() + return self.response(200, message="OK") + else: + return self.response_404() except SSHTunnelDeleteFailedError as ex: logger.error( "Error deleting SSH Tunnel %s: %s", From 9e8467fc24488c0e1bb221e9d1f94c3ff6ed2953 Mon Sep 17 00:00:00 2001 From: geido Date: Thu, 15 Feb 2024 19:58:38 +0200 Subject: [PATCH 2/6] Add breaking change --- UPDATING.md | 1 + superset/databases/api.py | 4 +--- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/UPDATING.md b/UPDATING.md index 8f5785ac8c97d..4dd68340bcdb4 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -30,6 +30,7 @@ assists people when migrating to a new version. ### Breaking Changes +- [27130](https://github.com/apache/superset/pull/27130): Fixes the DELETE `/database/{id}/ssh_tunnel/`` endpoint to now correctly accept a database ID as a parameter, rather than an SSH tunnel ID. - [27117](https://github.com/apache/superset/pull/27117): Removes the following deprecated endpoints: `/superset/sqllab`, `/superset/sqllab/history`, `/sqllab/my_queries` use `/sqllab`, `/sqllab/history`, `/savedqueryview/list/?_flt_0_user={get_user_id()}` instead. - [26347](https://github.com/apache/superset/issues/26347): Removes the deprecated `VERSIONED_EXPORT` feature flag. The previous value of the feature flag was `True` and now the feature is permanently enabled. - [26328](https://github.com/apache/superset/issues/26328): Removes the deprecated Filter Box code and it's associated dependencies `react-select` and `array-move`. It also removes the `DeprecatedSelect` and `AsyncSelect` components that were exclusively used by filter boxes. Existing filter boxes will be automatically migrated to native filters. diff --git a/superset/databases/api.py b/superset/databases/api.py index f251b45880769..14b5dd38a9409 100644 --- a/superset/databases/api.py +++ b/superset/databases/api.py @@ -48,7 +48,6 @@ from superset.commands.database.ssh_tunnel.exceptions import ( SSHTunnelDeleteFailedError, SSHTunnelingNotEnabledError, - SSHTunnelNotFoundError, ) from superset.commands.database.tables import TablesDatabaseCommand from superset.commands.database.test_connection import TestConnectionDatabaseCommand @@ -1492,8 +1491,7 @@ def delete_ssh_tunnel(self, pk: int) -> Response: if existing_ssh_tunnel_model: DeleteSSHTunnelCommand(existing_ssh_tunnel_model.id).run() return self.response(200, message="OK") - else: - return self.response_404() + return self.response_404() except SSHTunnelDeleteFailedError as ex: logger.error( "Error deleting SSH Tunnel %s: %s", From 40543b51cbe876a5cb399df6e6ca16eff446c23f Mon Sep 17 00:00:00 2001 From: geido Date: Thu, 15 Feb 2024 20:06:26 +0200 Subject: [PATCH 3/6] Update test --- tests/unit_tests/databases/api_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit_tests/databases/api_test.py b/tests/unit_tests/databases/api_test.py index f867f82a98d8c..5925362aa5b07 100644 --- a/tests/unit_tests/databases/api_test.py +++ b/tests/unit_tests/databases/api_test.py @@ -458,7 +458,7 @@ def test_delete_ssh_tunnel( assert 1 == response_tunnel.database_id # Delete the recently created SSHTunnel - response_delete_tunnel = client.delete("/api/v1/database/1/ssh_tunnel/") + response_delete_tunnel = client.delete(f"/api/v1/database/{database.id}/ssh_tunnel/") assert response_delete_tunnel.json["message"] == "OK" response_tunnel = DatabaseDAO.get_ssh_tunnel(1) From c71c4ef13d4c0e79240cea1d23fe22b2743b6f57 Mon Sep 17 00:00:00 2001 From: geido Date: Thu, 15 Feb 2024 20:17:11 +0200 Subject: [PATCH 4/6] Lint --- tests/unit_tests/databases/api_test.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/unit_tests/databases/api_test.py b/tests/unit_tests/databases/api_test.py index 5925362aa5b07..c0e1723fd8361 100644 --- a/tests/unit_tests/databases/api_test.py +++ b/tests/unit_tests/databases/api_test.py @@ -458,7 +458,9 @@ def test_delete_ssh_tunnel( assert 1 == response_tunnel.database_id # Delete the recently created SSHTunnel - response_delete_tunnel = client.delete(f"/api/v1/database/{database.id}/ssh_tunnel/") + response_delete_tunnel = client.delete( + f"/api/v1/database/{database.id}/ssh_tunnel/" + ) assert response_delete_tunnel.json["message"] == "OK" response_tunnel = DatabaseDAO.get_ssh_tunnel(1) From 4e4cb0f6acb60d2dd0375183720f931d5a71eb6c Mon Sep 17 00:00:00 2001 From: geido Date: Thu, 15 Feb 2024 20:46:53 +0200 Subject: [PATCH 5/6] Deprecate endpoint --- superset/databases/api.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/superset/databases/api.py b/superset/databases/api.py index 14b5dd38a9409..abf40488ff8fb 100644 --- a/superset/databases/api.py +++ b/superset/databases/api.py @@ -22,6 +22,7 @@ from typing import Any, cast, Optional from zipfile import is_zipfile, ZipFile +from deprecation import deprecated from flask import request, Response, send_file from flask_appbuilder.api import expose, protect, rison, safe from flask_appbuilder.models.sqla.interface import SQLAInterface @@ -1446,6 +1447,7 @@ def validate_parameters(self) -> FlaskResponse: @expose("//ssh_tunnel/", methods=("DELETE",)) @protect() @statsd_metrics + @deprecated(deprecated_in="5.0") @event_logger.log_this_with_context( action=lambda self, *args, **kwargs: f"{self.__class__.__name__}" f".delete_ssh_tunnel", From 99237e3a8e6540dffdfdafef053f81553b892dfb Mon Sep 17 00:00:00 2001 From: Geido <60598000+geido@users.noreply.github.com> Date: Thu, 15 Feb 2024 20:54:16 +0200 Subject: [PATCH 6/6] Update superset/databases/api.py Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> --- superset/databases/api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/databases/api.py b/superset/databases/api.py index abf40488ff8fb..2f95bd0442754 100644 --- a/superset/databases/api.py +++ b/superset/databases/api.py @@ -1447,7 +1447,7 @@ def validate_parameters(self) -> FlaskResponse: @expose("//ssh_tunnel/", methods=("DELETE",)) @protect() @statsd_metrics - @deprecated(deprecated_in="5.0") + @deprecated(deprecated_in="4.0") @event_logger.log_this_with_context( action=lambda self, *args, **kwargs: f"{self.__class__.__name__}" f".delete_ssh_tunnel",