You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2022/11/22 18:59:06 UTC
[GitHub] [superset] Antonio-RiveroMartnez opened a new pull request, #22199: feat(ssh_tunnel): APIs for SSH Tunnels
Antonio-RiveroMartnez opened a new pull request, #22199:
URL: https://github.com/apache/superset/pull/22199
### SUMMARY
Add our CREATE, UPDATE and DELETE APIs for SSH Tunnels.
- [ ] CREATE API
> - [X] API endpoint calling the command
> - [X] Happy path tests
> - [ ] Error path tests
- [ ] UPDATE API
> - [ ] API endpoint calling the command
> - [ ] Happy path tests
> - [ ] Error path tests
- [ ] DELETE API
> - [X] API endpoint calling the command
> - [X] Happy path tests
> - [ ] Error path tests
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh commented on a diff in pull request #22199: feat(ssh_tunnel): APIs for SSH Tunnels
Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #22199:
URL: https://github.com/apache/superset/pull/22199#discussion_r1037411961
##########
superset/databases/api.py:
##########
@@ -1204,3 +1210,58 @@ def validate_parameters(self) -> FlaskResponse:
command = ValidateDatabaseParametersCommand(payload)
command.run()
return self.response(200, message="OK")
+
+ @expose("/<int:pk>/ssh_tunnel/", methods=["DELETE"])
+ @protect()
+ @safe
Review Comment:
+1 we should remove this so SIP-41 errors can be returned
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] Antonio-RiveroMartnez closed pull request #22199: feat(ssh_tunnel): APIs for SSH Tunnels
Posted by GitBox <gi...@apache.org>.
Antonio-RiveroMartnez closed pull request #22199: feat(ssh_tunnel): APIs for SSH Tunnels
URL: https://github.com/apache/superset/pull/22199
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh commented on a diff in pull request #22199: feat(ssh_tunnel): APIs for SSH Tunnels
Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #22199:
URL: https://github.com/apache/superset/pull/22199#discussion_r1033852119
##########
superset/databases/schemas.py:
##########
@@ -365,6 +365,26 @@ class Meta: # pylint: disable=too-few-public-methods
)
+class DatabaseSSHTunnel(Schema):
+ id = fields.Integer()
+ database_id = fields.Integer()
+
+ server_address = fields.String()
+ server_port = fields.Integer()
+ username = fields.String()
+
+ # Basic Authentication
+ password = fields.String(required=False)
+
+ # password protected private key authentication
+ private_key = fields.String(required=False)
+ private_key_password = fields.String(required=False)
+
+ # remote binding port
+ bind_host = fields.String()
+ bind_port = fields.Integer()
Review Comment:
we don't need to add this part to the schema
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh commented on a diff in pull request #22199: feat(ssh_tunnel): APIs for SSH Tunnels
Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #22199:
URL: https://github.com/apache/superset/pull/22199#discussion_r1044646751
##########
superset/models/core.py:
##########
@@ -350,6 +350,19 @@ def set_sqlalchemy_uri(self, uri: str) -> None:
conn = conn.set(password=PASSWORD_MASK if conn.password else None)
self.sqlalchemy_uri = str(conn) # hides the password
+ def mask_password_in_ssh_tunnel(self, ssh_tunnel: "SSHTunnel") -> None:
+ if ssh_tunnel.password is not None and ssh_tunnel.password != PASSWORD_MASK:
+ # Mask the real password with our password mask
+ self.ssh_tunnel["password"] = PASSWORD_MASK # pylint: disable=no-member
+ if (
+ ssh_tunnel.private_key_password is not None
+ and ssh_tunnel.private_key_password != PASSWORD_MASK
+ ):
+ # Mask the real password with our password mask
+ self.ssh_tunnel[ # pylint: disable=no-member
+ "private_key_password"
+ ] = PASSWORD_MASK
Review Comment:
this isn't overwriting the actual value of the password before saving right?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] github-actions[bot] commented on pull request #22199: feat(ssh_tunnel): APIs for SSH Tunnels
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #22199:
URL: https://github.com/apache/superset/pull/22199#issuecomment-1334134832
Ephemeral environment shutdown and build artifacts deleted.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] commented on pull request #22199: [DRAFT] feat(ssh_tunnel): APIs for SSH Tunnels
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #22199:
URL: https://github.com/apache/superset/pull/22199#issuecomment-1324133266
# [Codecov](https://codecov.io/gh/apache/superset/pull/22199?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#22199](https://codecov.io/gh/apache/superset/pull/22199?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (383686b) into [create-sshtunnelconfig-tbl](https://codecov.io/gh/apache/superset/commit/6c59663638e0293da4f243dcb8f13f5a071a79ef?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6c59663) will **decrease** coverage by `2.10%`.
> The diff coverage is `70.58%`.
> :exclamation: Current head 383686b differs from pull request most recent head ba164aa. Consider uploading reports for the commit ba164aa to get more accurate results
```diff
@@ Coverage Diff @@
## create-sshtunnelconfig-tbl #22199 +/- ##
==============================================================
- Coverage 67.00% 64.89% -2.11%
==============================================================
Files 1841 1840 -1
Lines 70087 70084 -3
Branches 7590 7590
==============================================================
- Hits 46959 45479 -1480
- Misses 21162 22639 +1477
Partials 1966 1966
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `52.63% <70.58%> (+0.04%)` | :arrow_up: |
| mysql | `?` | |
| postgres | `?` | |
| presto | `52.53% <70.58%> (+0.04%)` | :arrow_up: |
| python | `76.94% <70.58%> (-4.41%)` | :arrow_down: |
| sqlite | `76.56% <70.58%> (-0.02%)` | :arrow_down: |
| unit | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/22199?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset/constants.py](https://codecov.io/gh/apache/superset/pull/22199/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29uc3RhbnRzLnB5) | `100.00% <ø> (ø)` | |
| [superset/databases/api.py](https://codecov.io/gh/apache/superset/pull/22199/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2FwaS5weQ==) | `84.21% <52.94%> (-9.33%)` | :arrow_down: |
| [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/22199/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `60.34% <60.00%> (-26.45%)` | :arrow_down: |
| [superset/databases/schemas.py](https://codecov.io/gh/apache/superset/pull/22199/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YWJhc2VzL3NjaGVtYXMucHk=) | `91.44% <100.00%> (-6.91%)` | :arrow_down: |
| [...set/advanced\_data\_type/plugins/internet\_address.py](https://codecov.io/gh/apache/superset/pull/22199/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvYWR2YW5jZWRfZGF0YV90eXBlL3BsdWdpbnMvaW50ZXJuZXRfYWRkcmVzcy5weQ==) | `16.32% <0.00%> (-79.60%)` | :arrow_down: |
| [superset/utils/pandas\_postprocessing/boxplot.py](https://codecov.io/gh/apache/superset/pull/22199/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvcGFuZGFzX3Bvc3Rwcm9jZXNzaW5nL2JveHBsb3QucHk=) | `20.51% <0.00%> (-79.49%)` | :arrow_down: |
| [superset/charts/post\_processing.py](https://codecov.io/gh/apache/superset/pull/22199/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY2hhcnRzL3Bvc3RfcHJvY2Vzc2luZy5weQ==) | `11.76% <0.00%> (-77.95%)` | :arrow_down: |
| [...perset/advanced\_data\_type/plugins/internet\_port.py](https://codecov.io/gh/apache/superset/pull/22199/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvYWR2YW5jZWRfZGF0YV90eXBlL3BsdWdpbnMvaW50ZXJuZXRfcG9ydC5weQ==) | `18.75% <0.00%> (-77.09%)` | :arrow_down: |
| [superset/datasets/commands/create.py](https://codecov.io/gh/apache/superset/pull/22199/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvY3JlYXRlLnB5) | `30.61% <0.00%> (-69.39%)` | :arrow_down: |
| [superset/datasets/commands/update.py](https://codecov.io/gh/apache/superset/pull/22199/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvdXBkYXRlLnB5) | `25.00% <0.00%> (-69.05%)` | :arrow_down: |
| ... and [91 more](https://codecov.io/gh/apache/superset/pull/22199/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a diff in pull request #22199: feat(ssh_tunnel): APIs for SSH Tunnels
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #22199:
URL: https://github.com/apache/superset/pull/22199#discussion_r1035431290
##########
tests/integration_tests/databases/api_tests.py:
##########
@@ -279,6 +281,262 @@ def test_create_database(self):
db.session.delete(model)
db.session.commit()
+ @mock.patch(
+ "superset.databases.commands.test_connection.TestConnectionDatabaseCommand.run",
+ )
+ def test_create_database_with_ssh_tunnel(
+ self, mock_test_connection_database_command_run
+ ):
+ """
+ Database API: Test create with SSH Tunnel
+ """
+ self.login(username="admin")
+ example_db = get_example_database()
+ if example_db.backend == "sqlite":
+ return
+ ssh_tunnel_properties = {
+ "server_address": "123.132.123.1",
+ "server_port": 8080,
+ "username": "foo",
+ "password": "bar",
+ }
+ database_data = {
+ "database_name": "test-db-with-ssh-tunnel",
+ "sqlalchemy_uri": example_db.sqlalchemy_uri_decrypted,
+ "ssh_tunnel": ssh_tunnel_properties,
+ }
+
+ uri = "api/v1/database/"
+ rv = self.client.post(uri, json=database_data)
+ response = json.loads(rv.data.decode("utf-8"))
+ self.assertEqual(rv.status_code, 201)
+ model_ssh_tunnel = (
+ db.session.query(SSHTunnel)
+ .filter(SSHTunnel.database_id == response.get("id"))
+ .one()
+ )
+ self.assertEqual(model_ssh_tunnel.database_id, response.get("id"))
+ # Cleanup
+ model = db.session.query(Database).get(response.get("id"))
+ db.session.delete(model)
+ db.session.commit()
+
+ @mock.patch(
+ "superset.databases.commands.test_connection.TestConnectionDatabaseCommand.run",
+ )
+ @mock.patch(
+ "superset.models.core.Database.get_all_schema_names",
+ )
+ def test_update_database_with_ssh_tunnel(
+ self, mock_test_connection_database_command_run, mock_get_all_schema_names
+ ):
+ """
+ Database API: Test update with SSH Tunnel
+ """
+ self.login(username="admin")
+ example_db = get_example_database()
+ if example_db.backend == "sqlite":
+ return
+ ssh_tunnel_properties = {
+ "server_address": "123.132.123.1",
+ "server_port": 8080,
+ "username": "foo",
+ "password": "bar",
+ }
+ database_data = {
+ "database_name": "test-db-with-ssh-tunnel",
+ "sqlalchemy_uri": example_db.sqlalchemy_uri_decrypted,
+ }
+ database_data_with_ssh_tunnel = {
+ "database_name": "test-db-with-ssh-tunnel",
+ "sqlalchemy_uri": example_db.sqlalchemy_uri_decrypted,
+ "ssh_tunnel": ssh_tunnel_properties,
+ }
+
+ uri = "api/v1/database/"
+ rv = self.client.post(uri, json=database_data)
+ response = json.loads(rv.data.decode("utf-8"))
+ self.assertEqual(rv.status_code, 201)
+
+ uri = "api/v1/database/{}".format(response.get("id"))
+ rv = self.client.put(uri, json=database_data_with_ssh_tunnel)
+ response_update = json.loads(rv.data.decode("utf-8"))
+ self.assertEqual(rv.status_code, 200)
+
+ model_ssh_tunnel = (
+ db.session.query(SSHTunnel)
+ .filter(SSHTunnel.database_id == response_update.get("id"))
+ .one()
+ )
+ self.assertEqual(model_ssh_tunnel.database_id, response_update.get("id"))
+ # Cleanup
+ model = db.session.query(Database).get(response.get("id"))
+ db.session.delete(model)
+ db.session.commit()
+
+ @mock.patch(
+ "superset.databases.commands.test_connection.TestConnectionDatabaseCommand.run",
+ )
+ def test_update_ssh_tunnel_via_database_api(
+ self, mock_test_connection_database_command_run
+ ):
+ """
+ Database API: Test update with SSH Tunnel
+ """
+ self.login(username="admin")
+ example_db = get_example_database()
+
+ if example_db.backend == "sqlite":
+ return
+ initial_ssh_tunnel_properties = {
+ "server_address": "123.132.123.1",
+ "server_port": 8080,
+ "username": "foo",
+ "password": "bar",
+ }
+ updated_ssh_tunnel_properties = {
+ "username": "Test",
+ }
+ database_data_with_ssh_tunnel = {
+ "database_name": "test-db-with-ssh-tunnel",
+ "sqlalchemy_uri": example_db.sqlalchemy_uri_decrypted,
+ "ssh_tunnel": initial_ssh_tunnel_properties,
+ }
+ database_data_with_ssh_tunnel_update = {
+ "database_name": "test-db-with-ssh-tunnel",
+ "sqlalchemy_uri": example_db.sqlalchemy_uri_decrypted,
+ "ssh_tunnel": updated_ssh_tunnel_properties,
+ }
+
+ uri = "api/v1/database/"
+ rv = self.client.post(uri, json=database_data_with_ssh_tunnel)
+ response = json.loads(rv.data.decode("utf-8"))
+ self.assertEqual(rv.status_code, 201)
+ model_ssh_tunnel = (
+ db.session.query(SSHTunnel)
+ .filter(SSHTunnel.database_id == response.get("id"))
+ .one()
+ )
+ self.assertEqual(model_ssh_tunnel.database_id, response.get("id"))
+ self.assertEqual(model_ssh_tunnel.username, "foo")
+ with mock.patch(
+ "superset.models.core.Database.get_all_schema_names",
+ return_value=["information_schema", "public"],
+ ):
+ uri = "api/v1/database/{}".format(response.get("id"))
+ rv = self.client.put(uri, json=database_data_with_ssh_tunnel_update)
+ response_update = json.loads(rv.data.decode("utf-8"))
+ self.assertEqual(rv.status_code, 200)
+ model_ssh_tunnel = (
+ db.session.query(SSHTunnel)
+ .filter(SSHTunnel.database_id == response_update.get("id"))
+ .one()
+ )
+ self.assertEqual(model_ssh_tunnel.database_id, response_update.get("id"))
+ self.assertEqual(model_ssh_tunnel.username, "Test")
+ self.assertEqual(model_ssh_tunnel.server_address, "123.132.123.1")
+ self.assertEqual(model_ssh_tunnel.server_port, 8080)
+ # Cleanup
+ model = db.session.query(Database).get(response.get("id"))
+ db.session.delete(model)
+ db.session.commit()
+
+ @mock.patch(
+ "superset.databases.commands.test_connection.TestConnectionDatabaseCommand.run",
+ )
+ def test_cascade_delete_ssh_tunnel(self, mock_test_connection_database_command_run):
+ """
+ Database API: Test create with SSH Tunnel
+ """
+ self.login(username="admin")
+ example_db = get_example_database()
+ if example_db.backend == "sqlite":
+ return
+ ssh_tunnel_properties = {
+ "server_address": "123.132.123.1",
+ "server_port": 8080,
+ "username": "foo",
+ "password": "bar",
+ }
+ database_data = {
+ "database_name": "test-db-with-ssh-tunnel",
+ "sqlalchemy_uri": example_db.sqlalchemy_uri_decrypted,
+ "ssh_tunnel": ssh_tunnel_properties,
+ }
+
+ uri = "api/v1/database/"
+ rv = self.client.post(uri, json=database_data)
+ response = json.loads(rv.data.decode("utf-8"))
+ self.assertEqual(rv.status_code, 201)
+ model_ssh_tunnel = (
+ db.session.query(SSHTunnel)
+ .filter(SSHTunnel.database_id == response.get("id"))
+ .one()
+ )
+ self.assertEqual(model_ssh_tunnel.database_id, response.get("id"))
+ # Cleanup
+ model = db.session.query(Database).get(response.get("id"))
+ db.session.delete(model)
+ db.session.commit()
+ model_ssh_tunnel = (
+ db.session.query(SSHTunnel)
+ .filter(SSHTunnel.database_id == response.get("id"))
+ .one_or_none()
+ )
+ assert model_ssh_tunnel is None
Review Comment:
nice!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] Antonio-RiveroMartnez closed pull request #22199: feat(ssh_tunnel): APIs for SSH Tunnels
Posted by GitBox <gi...@apache.org>.
Antonio-RiveroMartnez closed pull request #22199: feat(ssh_tunnel): APIs for SSH Tunnels
URL: https://github.com/apache/superset/pull/22199
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] Antonio-RiveroMartnez commented on a diff in pull request #22199: feat(ssh_tunnel): APIs for SSH Tunnels
Posted by GitBox <gi...@apache.org>.
Antonio-RiveroMartnez commented on code in PR #22199:
URL: https://github.com/apache/superset/pull/22199#discussion_r1060818784
##########
superset/databases/api.py:
##########
@@ -280,6 +327,12 @@ def post(self) -> FlaskResponse:
if new_model.driver:
item["driver"] = new_model.driver
+ # Return SSH Tunnel and hide passwords if any
+ if item.get("ssh_tunnel"):
+ item["ssh_tunnel"] = new_model.ssh_tunnel # pylint: disable=no-member
Review Comment:
Sure! I'll indeed use our `mask` as I did here: https://github.com/apache/superset/pull/22513/files#diff-f9deb9b63f5f2db38b811ff092cf90b0d4d558223f743bbbb5b203c203d2d590R336 but sure, let me add that util. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a diff in pull request #22199: feat(ssh_tunnel): APIs for SSH Tunnels
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #22199:
URL: https://github.com/apache/superset/pull/22199#discussion_r1035432633
##########
superset/databases/commands/update.py:
##########
@@ -94,11 +95,35 @@ def run(self) -> Model:
security_manager.add_permission_view_menu(
"schema_access", security_manager.get_schema_perm(database, schema)
)
+
+ if ssh_tunnel_properties := self._properties.get("ssh_tunnel"):
+ existing_ssh_tunnel = DatabaseDAO.get_ssh_tunnel(database.id)
+ if existing_ssh_tunnel is None:
+ # We couldn't found an existing tunnel so we need to create one
+ SSHTunnelDAO.create(
+ {
+ **ssh_tunnel_properties,
+ "database_id": database.id,
+ },
+ commit=False,
+ )
+ else:
+ # We found an existing tunnel so we need to update it
+ ssh_tunnel_model = SSHTunnelDAO.find_by_id(existing_ssh_tunnel.id)
Review Comment:
wouldn't this lookup/instance here be the same as the existing_ssh_tunnel if it were not None? In other words, why do we have to fetch it again?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] Antonio-RiveroMartnez commented on a diff in pull request #22199: feat(ssh_tunnel): APIs for SSH Tunnels
Posted by GitBox <gi...@apache.org>.
Antonio-RiveroMartnez commented on code in PR #22199:
URL: https://github.com/apache/superset/pull/22199#discussion_r1036202956
##########
superset/databases/commands/update.py:
##########
@@ -94,11 +95,35 @@ def run(self) -> Model:
security_manager.add_permission_view_menu(
"schema_access", security_manager.get_schema_perm(database, schema)
)
+
+ if ssh_tunnel_properties := self._properties.get("ssh_tunnel"):
+ existing_ssh_tunnel = DatabaseDAO.get_ssh_tunnel(database.id)
+ if existing_ssh_tunnel is None:
+ # We couldn't found an existing tunnel so we need to create one
+ SSHTunnelDAO.create(
+ {
+ **ssh_tunnel_properties,
+ "database_id": database.id,
+ },
+ commit=False,
+ )
+ else:
+ # We found an existing tunnel so we need to update it
+ ssh_tunnel_model = SSHTunnelDAO.find_by_id(existing_ssh_tunnel.id)
Review Comment:
leftover from when it was a dict. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh merged pull request #22199: feat(ssh_tunnel): APIs for SSH Tunnels
Posted by GitBox <gi...@apache.org>.
hughhhh merged PR #22199:
URL: https://github.com/apache/superset/pull/22199
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] Antonio-RiveroMartnez commented on a diff in pull request #22199: feat(ssh_tunnel): APIs for SSH Tunnels
Posted by GitBox <gi...@apache.org>.
Antonio-RiveroMartnez commented on code in PR #22199:
URL: https://github.com/apache/superset/pull/22199#discussion_r1043457855
##########
superset/databases/commands/update.py:
##########
@@ -94,10 +95,31 @@ def run(self) -> Model:
security_manager.add_permission_view_menu(
"schema_access", security_manager.get_schema_perm(database, schema)
)
+
+ if ssh_tunnel_properties := self._properties.get("ssh_tunnel"):
+ 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
+ ssh_tunnel = SSHTunnelDAO.create(
Review Comment:
Should we instead try to use the `SSHTunnelCommands` and not the DAOs directly? @hughhhh @eschutho I mean, if not, then `CreateSSHTunnelCommand` and `UpdateSSHTunnelCommand` would never be actually used, correct? Now, other question is, how do we manage the `commit=False` when calling a command inside another one? Or should we just keep using the DAOs and get rid of the Commands?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh commented on a diff in pull request #22199: feat(ssh_tunnel): APIs for SSH Tunnels
Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #22199:
URL: https://github.com/apache/superset/pull/22199#discussion_r1060814477
##########
superset/databases/api.py:
##########
@@ -280,6 +327,12 @@ def post(self) -> FlaskResponse:
if new_model.driver:
item["driver"] = new_model.driver
+ # Return SSH Tunnel and hide passwords if any
+ if item.get("ssh_tunnel"):
+ item["ssh_tunnel"] = new_model.ssh_tunnel # pylint: disable=no-member
Review Comment:
create a util function that does the popping/deletion and returns the object without those values
##########
superset/databases/api.py:
##########
@@ -361,6 +414,11 @@ def put(self, pk: int) -> Response:
item["sqlalchemy_uri"] = changed_model.sqlalchemy_uri
if changed_model.parameters:
item["parameters"] = changed_model.parameters
+ # Return SSH Tunnel and hide passwords if any
+ if item.get("ssh_tunnel"):
+ item["ssh_tunnel"] = changed_model.ssh_tunnel
Review Comment:
use the util function here as well
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a diff in pull request #22199: feat(ssh_tunnel): APIs for SSH Tunnels
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #22199:
URL: https://github.com/apache/superset/pull/22199#discussion_r1035433490
##########
superset/databases/commands/update.py:
##########
@@ -94,11 +95,35 @@ def run(self) -> Model:
security_manager.add_permission_view_menu(
"schema_access", security_manager.get_schema_perm(database, schema)
)
+
+ if ssh_tunnel_properties := self._properties.get("ssh_tunnel"):
+ existing_ssh_tunnel = DatabaseDAO.get_ssh_tunnel(database.id)
+ if existing_ssh_tunnel is None:
+ # We couldn't found an existing tunnel so we need to create one
+ SSHTunnelDAO.create(
+ {
+ **ssh_tunnel_properties,
+ "database_id": database.id,
+ },
+ commit=False,
+ )
+ else:
+ # We found an existing tunnel so we need to update it
+ ssh_tunnel_model = SSHTunnelDAO.find_by_id(existing_ssh_tunnel.id)
+ SSHTunnelDAO.update(
+ ssh_tunnel_model,
+ ssh_tunnel_properties,
+ commit=False,
+ )
+
db.session.commit()
except DAOUpdateFailedError as ex:
logger.exception(ex.exception)
raise DatabaseUpdateFailedError() from ex
+ except DAOCreateFailedError as ex:
+ logger.exception(ex.exception)
Review Comment:
nit, but if the except block on both of these errors, is the same, you might as well just catch both errors in the same block, no?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a diff in pull request #22199: feat(ssh_tunnel): APIs for SSH Tunnels
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #22199:
URL: https://github.com/apache/superset/pull/22199#discussion_r1035432633
##########
superset/databases/commands/update.py:
##########
@@ -94,11 +95,35 @@ def run(self) -> Model:
security_manager.add_permission_view_menu(
"schema_access", security_manager.get_schema_perm(database, schema)
)
+
+ if ssh_tunnel_properties := self._properties.get("ssh_tunnel"):
+ existing_ssh_tunnel = DatabaseDAO.get_ssh_tunnel(database.id)
+ if existing_ssh_tunnel is None:
+ # We couldn't found an existing tunnel so we need to create one
+ SSHTunnelDAO.create(
+ {
+ **ssh_tunnel_properties,
+ "database_id": database.id,
+ },
+ commit=False,
+ )
+ else:
+ # We found an existing tunnel so we need to update it
+ ssh_tunnel_model = SSHTunnelDAO.find_by_id(existing_ssh_tunnel.id)
Review Comment:
wouldn't this lookup/instance here be the same as the existing_ssh_tunnel if it were not None?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a diff in pull request #22199: feat(ssh_tunnel): APIs for SSH Tunnels
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #22199:
URL: https://github.com/apache/superset/pull/22199#discussion_r1035429957
##########
superset/databases/api.py:
##########
@@ -1204,3 +1210,58 @@ def validate_parameters(self) -> FlaskResponse:
command = ValidateDatabaseParametersCommand(payload)
command.run()
return self.response(200, message="OK")
+
+ @expose("/<int:pk>/ssh_tunnel/", methods=["DELETE"])
+ @protect()
+ @safe
Review Comment:
IIRC, we no longer want to use this decorator because we want all errors to raise to be SIP41 compliant. DId I get that right @betodealmeida?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a diff in pull request #22199: feat(ssh_tunnel): APIs for SSH Tunnels
Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #22199:
URL: https://github.com/apache/superset/pull/22199#discussion_r1035433160
##########
superset/databases/commands/update.py:
##########
@@ -94,11 +95,35 @@ def run(self) -> Model:
security_manager.add_permission_view_menu(
"schema_access", security_manager.get_schema_perm(database, schema)
)
+
+ if ssh_tunnel_properties := self._properties.get("ssh_tunnel"):
+ existing_ssh_tunnel = DatabaseDAO.get_ssh_tunnel(database.id)
+ if existing_ssh_tunnel is None:
+ # We couldn't found an existing tunnel so we need to create one
+ SSHTunnelDAO.create(
+ {
+ **ssh_tunnel_properties,
+ "database_id": database.id,
+ },
+ commit=False,
+ )
+ else:
+ # We found an existing tunnel so we need to update it
+ ssh_tunnel_model = SSHTunnelDAO.find_by_id(existing_ssh_tunnel.id)
+ SSHTunnelDAO.update(
+ ssh_tunnel_model,
+ ssh_tunnel_properties,
+ commit=False,
+ )
+
db.session.commit()
except DAOUpdateFailedError as ex:
logger.exception(ex.exception)
raise DatabaseUpdateFailedError() from ex
+ except DAOCreateFailedError as ex:
+ logger.exception(ex.exception)
Review Comment:
the logger should log at the root api level
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] Antonio-RiveroMartnez commented on a diff in pull request #22199: feat(ssh_tunnel): APIs for SSH Tunnels
Posted by GitBox <gi...@apache.org>.
Antonio-RiveroMartnez commented on code in PR #22199:
URL: https://github.com/apache/superset/pull/22199#discussion_r1043457855
##########
superset/databases/commands/update.py:
##########
@@ -94,10 +95,31 @@ def run(self) -> Model:
security_manager.add_permission_view_menu(
"schema_access", security_manager.get_schema_perm(database, schema)
)
+
+ if ssh_tunnel_properties := self._properties.get("ssh_tunnel"):
+ 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
+ ssh_tunnel = SSHTunnelDAO.create(
Review Comment:
Should we instead try to use the `SSHTunnelCommands` and not the DAOs directly? @hughhhh @eschutho I mean, if not, then `CreateSSHTunnel` and `UpdateSSHTunnelCommand` would never be actually used, correct? Now, other question is, how do we manage the `commit=False` when calling a command inside another one? Or should we just keep using the DAOs and get rid of the Commands?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh commented on a diff in pull request #22199: feat(ssh_tunnel): APIs for SSH Tunnels
Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #22199:
URL: https://github.com/apache/superset/pull/22199#discussion_r1060822143
##########
superset/databases/commands/create.py:
##########
@@ -71,17 +75,26 @@ def run(self) -> Model:
try:
database = DatabaseDAO.create(self._properties, commit=False)
database.set_sqlalchemy_uri(database.sqlalchemy_uri)
- db.session.flush()
ssh_tunnel = None
if ssh_tunnel_properties := self._properties.get("ssh_tunnel"):
- ssh_tunnel = SSHTunnelDAO.create(
- {
- **ssh_tunnel_properties,
- "database_id": database.id,
- },
- commit=False,
- )
+ try:
+ # So database.id is not None
+ db.session.flush()
+ CreateSSHTunnelCommand(database.id, ssh_tunnel_properties).run()
Review Comment:
we need to have a reference for `ssh_tunnel` after it's created so it can pull down all the schema
https://github.com/apache/superset/pull/22199/files#diff-c3f1c5afb1b73f705f01bfa24cb4ca2ac66b84f82875abd3ddb73cb8147ea77eR100
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] Antonio-RiveroMartnez commented on a diff in pull request #22199: feat(ssh_tunnel): APIs for SSH Tunnels
Posted by GitBox <gi...@apache.org>.
Antonio-RiveroMartnez commented on code in PR #22199:
URL: https://github.com/apache/superset/pull/22199#discussion_r1060826700
##########
superset/databases/commands/create.py:
##########
@@ -71,17 +75,26 @@ def run(self) -> Model:
try:
database = DatabaseDAO.create(self._properties, commit=False)
database.set_sqlalchemy_uri(database.sqlalchemy_uri)
- db.session.flush()
ssh_tunnel = None
if ssh_tunnel_properties := self._properties.get("ssh_tunnel"):
- ssh_tunnel = SSHTunnelDAO.create(
- {
- **ssh_tunnel_properties,
- "database_id": database.id,
- },
- commit=False,
- )
+ try:
+ # So database.id is not None
+ db.session.flush()
+ CreateSSHTunnelCommand(database.id, ssh_tunnel_properties).run()
Review Comment:
Good catch, let me add it there! Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh commented on a diff in pull request #22199: feat(ssh_tunnel): APIs for SSH Tunnels
Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #22199:
URL: https://github.com/apache/superset/pull/22199#discussion_r1034960893
##########
tests/integration_tests/databases/api_tests.py:
##########
@@ -35,6 +35,8 @@
Review Comment:
Can we add a test on `SSHTunnel` failure, to make sure we don't save the `Database` object as wel..
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] Antonio-RiveroMartnez commented on pull request #22199: feat(ssh_tunnel): APIs for SSH Tunnels
Posted by GitBox <gi...@apache.org>.
Antonio-RiveroMartnez commented on PR #22199:
URL: https://github.com/apache/superset/pull/22199#issuecomment-1329210986
In a list, even when I'm clicking outside each menu to open the next one, it doesn't close opened ones. i.e, I am getting into this scenario:
<img width="1634" alt="Screen Shot 2022-11-28 at 11 28 22" src="https://user-images.githubusercontent.com/38889534/204302949-a494c3e9-1d13-4e0d-b438-01911286dbd1.png">
And now, even when clicking outside, none of those get closed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh commented on a diff in pull request #22199: feat(ssh_tunnel): APIs for SSH Tunnels
Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #22199:
URL: https://github.com/apache/superset/pull/22199#discussion_r1044646751
##########
superset/models/core.py:
##########
@@ -350,6 +350,19 @@ def set_sqlalchemy_uri(self, uri: str) -> None:
conn = conn.set(password=PASSWORD_MASK if conn.password else None)
self.sqlalchemy_uri = str(conn) # hides the password
+ def mask_password_in_ssh_tunnel(self, ssh_tunnel: "SSHTunnel") -> None:
+ if ssh_tunnel.password is not None and ssh_tunnel.password != PASSWORD_MASK:
+ # Mask the real password with our password mask
+ self.ssh_tunnel["password"] = PASSWORD_MASK # pylint: disable=no-member
+ if (
+ ssh_tunnel.private_key_password is not None
+ and ssh_tunnel.private_key_password != PASSWORD_MASK
+ ):
+ # Mask the real password with our password mask
+ self.ssh_tunnel[ # pylint: disable=no-member
+ "private_key_password"
+ ] = PASSWORD_MASK
Review Comment:
this isn't overwriting the actual value of the password before saving right?
Is it possible for us to just remove the password information from the payload
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh commented on pull request #22199: feat(ssh_tunnel): APIs for SSH Tunnels
Posted by GitBox <gi...@apache.org>.
hughhhh commented on PR #22199:
URL: https://github.com/apache/superset/pull/22199#issuecomment-1332359279
/testenv up
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] github-actions[bot] commented on pull request #22199: feat(ssh_tunnel): APIs for SSH Tunnels
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #22199:
URL: https://github.com/apache/superset/pull/22199#issuecomment-1332361927
@hughhhh Ephemeral environment spinning up at http://35.90.30.149:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] Antonio-RiveroMartnez closed pull request #22199: feat(ssh_tunnel): APIs for SSH Tunnels
Posted by GitBox <gi...@apache.org>.
Antonio-RiveroMartnez closed pull request #22199: feat(ssh_tunnel): APIs for SSH Tunnels
URL: https://github.com/apache/superset/pull/22199
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh commented on a diff in pull request #22199: feat(ssh_tunnel): APIs for SSH Tunnels
Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #22199:
URL: https://github.com/apache/superset/pull/22199#discussion_r1060813247
##########
superset/databases/api.py:
##########
@@ -219,6 +225,47 @@ class DatabaseRestApi(BaseSupersetModelRestApi):
ValidateSQLResponse,
)
+ @expose("/<int:pk>", methods=["GET"])
+ @protect()
+ @safe
+ def get(self, pk: int, **kwargs: Any) -> Response:
+ """Get a database
+ ---
+ get:
+ description: >-
+ Get a database with its SSH Tunnel if any
Review Comment:
Get a database
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] Antonio-RiveroMartnez commented on a diff in pull request #22199: feat(ssh_tunnel): APIs for SSH Tunnels
Posted by GitBox <gi...@apache.org>.
Antonio-RiveroMartnez commented on code in PR #22199:
URL: https://github.com/apache/superset/pull/22199#discussion_r1044732598
##########
superset/models/core.py:
##########
@@ -350,6 +350,19 @@ def set_sqlalchemy_uri(self, uri: str) -> None:
conn = conn.set(password=PASSWORD_MASK if conn.password else None)
self.sqlalchemy_uri = str(conn) # hides the password
+ def mask_password_in_ssh_tunnel(self, ssh_tunnel: "SSHTunnel") -> None:
+ if ssh_tunnel.password is not None and ssh_tunnel.password != PASSWORD_MASK:
+ # Mask the real password with our password mask
+ self.ssh_tunnel["password"] = PASSWORD_MASK # pylint: disable=no-member
+ if (
+ ssh_tunnel.private_key_password is not None
+ and ssh_tunnel.private_key_password != PASSWORD_MASK
+ ):
+ # Mask the real password with our password mask
+ self.ssh_tunnel[ # pylint: disable=no-member
+ "private_key_password"
+ ] = PASSWORD_MASK
Review Comment:
Yeah, probably it's best to just getting rid of it in the response. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] hughhhh commented on a diff in pull request #22199: feat(ssh_tunnel): APIs for SSH Tunnels
Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #22199:
URL: https://github.com/apache/superset/pull/22199#discussion_r1043530815
##########
superset/databases/commands/update.py:
##########
@@ -94,10 +95,31 @@ def run(self) -> Model:
security_manager.add_permission_view_menu(
"schema_access", security_manager.get_schema_perm(database, schema)
)
+
+ if ssh_tunnel_properties := self._properties.get("ssh_tunnel"):
+ 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
+ ssh_tunnel = SSHTunnelDAO.create(
Review Comment:
yea we should refactor these to use the commands instead, intially when we were writing these we prolly followed the pattern of the DatabaseDAO in this class
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org