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.

Sign up for GitHub

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

Jump to bottom

chore: module scope should not require the app context #28378

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

chore: module scope should not require the app context #28378

wants to merge 2 commits into from

Commits on May 29, 2024

  1. chore: module scope should not require the app context 

    This is a major issue that's been plaguing the backend for a long time. Currently you can't simply run a simple `from superset import models` without getting an error about the app context missing.

This DRAFT PR tries to evaluate what's getting in the way. So far I've identified:

- app.config being referenced in module scope, this is mostly easy to fix for common configuration, where we can rely on flask.current_app and avoid module scope
- dynamic/configurable model: this seems to be the core issue, where say we let people configure their EncryptedField's type
- some reliance on SecurityManager, and SecurityManager.user_model used as a relationship, I think this can be worked around using sqlalchemy's `relationship("use-a-string-reference")` call
- ???
    @mistercrunch
    mistercrunch committed May 29, 2024
    Configuration menu
    Copy the full SHA
    d5bc508 View commit details
    Browse the repository at this point in the history

  2. minor fix

    @mistercrunch
    mistercrunch committed May 29, 2024
    Configuration menu
    Copy the full SHA
    e6eb823 View commit details
    Browse the repository at this point in the history