Skip to content

fix: using sqla session to persist permission view#20590

Closed
zephyring wants to merge 7 commits intoapache:masterfrom
preset-io:zef/sc34552/using_sqla_session_to_set_perm
Closed

fix: using sqla session to persist permission view#20590
zephyring wants to merge 7 commits intoapache:masterfrom
preset-io:zef/sc34552/using_sqla_session_to_set_perm

Conversation

@zephyring
Copy link

SUMMARY

  1. In security manager, set_perm, instead of using connection to update perm directly, we should use sqla session.
  2. This will enable proper callback function to be triggered for PermissionView object

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

@codecov
Copy link

codecov bot commented Jul 1, 2022

Codecov Report

Merging #20590 (760097d) into master (8a57a71) will decrease coverage by 11.81%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           master   #20590       +/-   ##
===========================================
- Coverage   66.80%   54.98%   -11.82%     
===========================================
  Files        1754     1754               
  Lines       65562    65560        -2     
  Branches     6935     6935               
===========================================
- Hits        43796    36047     -7749     
- Misses      20011    27758     +7747     
  Partials     1755     1755               
Flag Coverage Δ
hive 53.77% <100.00%> (-0.01%) ⬇️
mysql ?
postgres ?
presto 53.63% <100.00%> (-0.01%) ⬇️
python 58.36% <100.00%> (-24.50%) ⬇️
sqlite ?
unit 50.64% <33.33%> (-0.01%) ⬇️

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

Impacted Files Coverage Δ
superset/connectors/sqla/utils.py 80.68% <100.00%> (-7.96%) ⬇️
superset/security/manager.py 62.18% <100.00%> (-33.16%) ⬇️
superset/utils/dashboard_import_export.py 0.00% <0.00%> (-100.00%) ⬇️
superset/key_value/commands/upsert.py 0.00% <0.00%> (-89.59%) ⬇️
superset/key_value/commands/update.py 0.00% <0.00%> (-89.37%) ⬇️
superset/key_value/commands/delete.py 0.00% <0.00%> (-85.30%) ⬇️
superset/key_value/commands/delete_expired.py 0.00% <0.00%> (-80.77%) ⬇️
superset/dashboards/commands/importers/v0.py 15.62% <0.00%> (-76.25%) ⬇️
superset/datasets/commands/update.py 25.88% <0.00%> (-68.24%) ⬇️
superset/datasets/commands/create.py 30.18% <0.00%> (-67.93%) ⬇️
... and 277 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 8a57a71...760097d. Read the comment docs.

@zephyring zephyring closed this Jul 4, 2022
@zephyring zephyring deleted the zef/sc34552/using_sqla_session_to_set_perm branch July 4, 2022 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants