Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(ssh-tunnel): Move SSHManager to extensions pattern #22433

Merged
merged 6 commits into from
Dec 16, 2022

Conversation

hughhhh
Copy link
Member

@hughhhh hughhhh commented Dec 15, 2022

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • 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

local_bind_address = "127.0.0.1"

@classmethod
def mutator(cls, sqlalchemy_url: str, server: sshtunnel.SSHTunnelForwarder) -> str:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

naming: build_sqlalchemy_url

remote_bind_address: str,
local_bind_address: str,
**kwargs: Dict[str, Any],
) -> sshtunnel.SSHTunnelForwarder:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should just return a ContextManager

@classmethod
def create_tunnel(
cls,
ssh_address_or_host: str,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of breaking these fields out, just pass in the SSHConfig model object

@@ -109,6 +111,31 @@ def init_app(self, app: Flask) -> None:
app.wsgi_app = SupersetProfiler(app.wsgi_app, self.interval) # type: ignore


class SSHManager: # pylint: disable=too-few-public-methods
local_bind_address = "127.0.0.1"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should implement init_app and should be wired up by the AppInitializer

@codecov
Copy link

codecov bot commented Dec 15, 2022

Codecov Report

Merging #22433 (139399c) into create-sshtunnelconfig-tbl (06e115b) will increase coverage by 0.00%.
The diff coverage is 72.54%.

@@                     Coverage Diff                     @@
##           create-sshtunnelconfig-tbl   #22433   +/-   ##
===========================================================
  Coverage                       66.95%   66.96%           
===========================================================
  Files                            1852     1853    +1     
  Lines                           70512    70532   +20     
  Branches                         7689     7689           
===========================================================
+ Hits                            47213    47232   +19     
- Misses                          21305    21306    +1     
  Partials                         1994     1994           
Flag Coverage Δ
hive 52.61% <72.54%> (+0.02%) ⬆️
javascript 53.79% <ø> (ø)
mysql 78.07% <72.54%> (+0.01%) ⬆️
postgres 78.13% <72.54%> (+0.01%) ⬆️
presto 52.50% <72.54%> (+0.02%) ⬆️
python 81.36% <72.54%> (+<0.01%) ⬆️
sqlite 76.59% <72.54%> (+0.01%) ⬆️
unit 51.14% <72.54%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
superset/databases/ssh_tunnel/models.py 100.00% <ø> (+24.13%) ⬆️
superset/extensions/ssh.py 65.71% <65.71%> (ø)
superset/models/core.py 90.37% <77.77%> (+0.37%) ⬆️
superset/config.py 91.90% <100.00%> (+0.43%) ⬆️
superset/extensions/__init__.py 98.85% <100.00%> (+0.02%) ⬆️
superset/initialization/__init__.py 91.05% <100.00%> (+0.08%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@pull-request-size pull-request-size bot added size/L and removed size/M labels Dec 16, 2022
@hughhhh hughhhh force-pushed the sshmanager-2 branch 2 times, most recently from 307162a to 005bd35 Compare December 16, 2022 03:27
@hughhhh hughhhh merged commit 13ed50d into create-sshtunnelconfig-tbl Dec 16, 2022
@mistercrunch mistercrunch deleted the sshmanager-2 branch March 26, 2024 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants