fix: remove encryption from db params #16214

eschutho · 2021-08-11T23:41:17Z

SUMMARY

There are some errors on db connection ui when toggling on encryption, it would also get returned in the params field, and then would error when trying to turn off ssl with the toggle.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before:
https://user-images.githubusercontent.com/5186919/129117204-42cd6021-647d-4c33-a4d1-eff56130b63c.mov

After:
https://user-images.githubusercontent.com/5186919/129117801-eb56b5fb-4767-427a-9f1d-3096fd353eab.mov

TESTING INSTRUCTIONS

edit a database, and toggle on/off ssl

ADDITIONAL INFORMATION

Has associated issue:
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

eschutho · 2021-08-11T23:43:37Z

superset/db_engine_specs/base.py

+            key: value
+            for (key, value) in url.query.items()
+            if (key, value) not in cls.encryption_parameters.items()
+        }


feel free to suggest a dryer way of doing this (some repetition with below)

codecov · 2021-08-12T02:21:12Z

Codecov Report

Merging #16214 (f153982) into master (ccfc95f) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master   #16214      +/-   ##
==========================================
- Coverage   76.73%   76.72%   -0.02%     
==========================================
  Files         996      996              
  Lines       52999    53079      +80     
  Branches     6738     6738              
==========================================
+ Hits        40668    40724      +56     
- Misses      12102    12126      +24     
  Partials      229      229

Flag	Coverage Δ
hive	`81.27% <100.00%> (-0.05%)`	⬇️
mysql	`81.53% <100.00%> (-0.09%)`	⬇️
postgres	`81.59% <100.00%> (-0.05%)`	⬇️
presto	`81.39% <100.00%> (-0.04%)`	⬇️
python	`82.11% <100.00%> (-0.04%)`	⬇️
sqlite	`81.23% <100.00%> (-0.05%)`	⬇️

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

Impacted Files	Coverage Δ
superset/db_engine_specs/base.py	`88.39% <100.00%> (+0.02%)`	⬆️
superset/utils/profiler.py	`40.90% <0.00%> (-6.15%)`	⬇️
superset/examples/utils.py	`28.57% <0.00%> (-3.43%)`	⬇️
superset/typing.py	`97.77% <0.00%> (-2.23%)`	⬇️
superset/datasets/commands/importers/v1/utils.py	`58.97% <0.00%> (ø)`
superset/connectors/sqla/models.py	`89.70% <0.00%> (+0.01%)`	⬆️
superset/connectors/druid/models.py	`82.99% <0.00%> (+0.03%)`	⬆️
superset/utils/core.py	`89.10% <0.00%> (+0.25%)`	⬆️
superset/db_engine_specs/presto.py	`90.37% <0.00%> (+0.41%)`	⬆️
superset/cli.py	`52.42% <0.00%> (+0.44%)`	⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ccfc95f...f153982. Read the comment docs.

superset/db_engine_specs/base.py

Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>

eschutho · 2021-08-12T18:08:33Z

/testenv up

github-actions · 2021-08-12T18:10:40Z

@eschutho Ephemeral environment spinning up at http://34.217.26.202:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

betodealmeida · 2021-08-12T18:43:43Z

superset/db_engine_specs/base.py

@@ -1433,6 +1434,11 @@ def get_parameters_from_uri(
        cls, uri: str, encrypted_extra: Optional[Dict[str, Any]] = None
    ) -> BasicParametersType:
        url = make_url(uri)
+        query = {


You can also do this:

query = dict(item for item in url.query.items() if item not in cls.encryption_parameters.items())

github-actions · 2021-08-12T19:03:57Z

Ephemeral environment shutdown and build artifacts deleted.

* remove encryption from db params * Update superset/db_engine_specs/base.py Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> (cherry picked from commit 67c4c01)

rosemarie-chiu · 2021-08-13T15:40:32Z

🏷 2021.31

* remove encryption from db params * Update superset/db_engine_specs/base.py Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> (cherry picked from commit 67c4c01)

…gies * upstream/master: (64 commits) check roles before fetching reports (#16260) chore: upgrade mypy and add type guards (#16227) fix: pivot columns with ints for name (#16259) chore(pylint): Bump Pylint to 2.9.6 (#16146) fix examples tab for dashboard (#16253) chore: bump superset-ui packages to 0.17.84 (#16251) chore: Shows the dataset description in the gallery dropdown (#16200) fix(Dashboard): Omnibar dropdown visibility and keyboard commands (#16168) chore: bump py version for integration test (#16213) fix: skip perms on query context update (#16250) refactor: external metadata fetch API (#16193) feat(dao): admin can remove self from object owners (#15149) fix(dashboard): cross filter chart highlight when filters badge icon clicked (#16233) fix: validate_parameters and query (#16241) fix: Remove Advanced Analytics tag for 2 charts (#16240) Revert "feat: Changing Dataset names (#16199)" (#16235) feat: Allow users to connect via legacy SQLA form (#16201) fix: remove encryption from db params (#16214) fix(Explore): Show the tooltip only when label does not fit the container in METRICS/FILTERS/GROUP BY/SORT BY of the DATA panel (#16060) Show/hide tooltips (#16192) ... # Conflicts: # superset/tasks/caching/cache_strategy.py

* remove encryption from db params * Update superset/db_engine_specs/base.py Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>

* remove encryption from db params * Update superset/db_engine_specs/base.py Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> (cherry picked from commit 67c4c01)

* remove encryption from db params * Update superset/db_engine_specs/base.py Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>

* remove encryption from db params * Update superset/db_engine_specs/base.py Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> (cherry picked from commit 67c4c01)

* remove encryption from db params * Update superset/db_engine_specs/base.py Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> Co-authored-by: Beto Dealmeida <roberto@dealmeida.net> (cherry picked from commit a91cfd2)

superset-github-bot bot added the preset-io label Aug 11, 2021

pull-request-size bot added the size/XS label Aug 11, 2021

eschutho changed the title ~~remove encryption from db params~~ fix: remove encryption from db params Aug 11, 2021

eschutho commented Aug 11, 2021

View reviewed changes

remove encryption from db params

77c650a

eschutho force-pushed the elizabeth/remove-encryption-from-params branch from f9c4641 to 77c650a Compare August 12, 2021 02:03

pull-request-size bot added size/S and removed size/XS labels Aug 12, 2021

eschutho requested a review from betodealmeida August 12, 2021 02:04

betodealmeida approved these changes Aug 12, 2021

View reviewed changes

superset/db_engine_specs/base.py Outdated Show resolved Hide resolved

superset/db_engine_specs/base.py Show resolved Hide resolved

Update superset/db_engine_specs/base.py

f153982

Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>

betodealmeida reviewed Aug 12, 2021

View reviewed changes

eschutho merged commit 67c4c01 into apache:master Aug 12, 2021

superset-github-bot bot added the preset:2021.31 label Aug 13, 2021

villebro added the v1.3 label Aug 16, 2021

mistercrunch added 🍒 1.3.0 🍒 1.3.1 labels Mar 13, 2024

mistercrunch added 🍒 1.3.2 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.4.0 labels Mar 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: remove encryption from db params #16214

fix: remove encryption from db params #16214

eschutho commented Aug 11, 2021 •

edited

eschutho Aug 11, 2021

codecov bot commented Aug 12, 2021 •

edited

eschutho commented Aug 12, 2021

github-actions bot commented Aug 12, 2021

betodealmeida Aug 12, 2021

github-actions bot commented Aug 12, 2021

rosemarie-chiu commented Aug 13, 2021

fix: remove encryption from db params #16214

fix: remove encryption from db params #16214

Conversation

eschutho commented Aug 11, 2021 • edited

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

eschutho Aug 11, 2021

Choose a reason for hiding this comment

codecov bot commented Aug 12, 2021 • edited

Codecov Report

eschutho commented Aug 12, 2021

github-actions bot commented Aug 12, 2021

betodealmeida Aug 12, 2021

Choose a reason for hiding this comment

github-actions bot commented Aug 12, 2021

rosemarie-chiu commented Aug 13, 2021

eschutho commented Aug 11, 2021 •

edited

codecov bot commented Aug 12, 2021 •

edited