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

refactor: Using self.get_session in security manager #10146

Conversation

john-bodley
Copy link
Member

SUMMARY

@dpgaspar per your #10034 (comment) I thought there was merit in replacing db.session with self.get_session throughout the entire security manager.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

CI.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Changes UI
  • Requires DB Migration.
  • Confirm DB Migration upgrade and downgrade tested.
  • Introduces new feature or API
  • Removes existing feature or API

Copy link
Member

@dpgaspar dpgaspar left a comment

Choose a reason for hiding this comment

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

LGTM, just a small comment

from superset import db

return db.session.query(self.role_model).filter_by(name="Public").first()
return self.get_session.query(self.role_model).filter_by(name="Public").first()
Copy link
Member

Choose a reason for hiding this comment

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

return self.get_session.query(self.role_model).filter_by(name="Public").one_or_none()

@john-bodley john-bodley force-pushed the john-bodley--refactor-security-manager-get-session branch from cbf4199 to 11c0aaa Compare July 4, 2020 19:25
@john-bodley john-bodley force-pushed the john-bodley--refactor-security-manager-get-session branch from 11c0aaa to 94a5740 Compare July 4, 2020 19:42
@john-bodley john-bodley merged commit 33584a8 into apache:master Jul 4, 2020
@john-bodley john-bodley deleted the john-bodley--refactor-security-manager-get-session branch July 4, 2020 20:04
auxten pushed a commit to auxten/incubator-superset that referenced this pull request Nov 20, 2020
Co-authored-by: John Bodley <john.bodley@airbnb.com>
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.37.0 labels Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/M 🚢 0.37.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants