Reload the DB URL/bind config when calling `db.create_all`

[greyli]

After 3.0, the engines is created in init_app, so there is no way to update the configuration after instantiating the extension object. However, we do need to update the config in some use cases. For example, in a simple application without using the app factory:

app = Flask(__name__)
app.config['SECRET_KEY'] = os.getenv('SECRET_KEY', 'secret string')
app.config['SQLALCHEMY_DATABASE_URI'] = os.getenv(
'DATABASE_URL', 'sqlite:////' + os.path.join(app.root_path, 'data.db')
)
db = SQLAlchemy(app)

In the test, you will want to use a different database URL, so you reset the config SQLALCHEMY_DATABASE_URI:

from app import app, db

class NotebookTestCase(unittest.TestCase):

    def setUp(self):
        self.context = app.test_request_context()
        self.context.push()
        self.client = app.test_client()
        app.config.update(
            TESTING=True,
            WTF_CSRF_ENABLED=False,
            SQLALCHEMY_DATABASE_URI='sqlite:///:memory:'  # <--
        )
        db.create_all()

It works in 2.x, but not 3.x.

I think we need to reload the config (SQLALCHEMY_DATABASE_URI and SQLALCHEMY_BINDS) before calling create_all, but it seems there is no way to update the engine URL after creating it.

[davidism]

This is intentional. Factory should create app, set config, then init extensions.

def create_app(test_config=None):
    app = Flask(__name__)
    app.config.from_mapping(default config values)
    
    if test_config is None:
        app.config.from_prefixed_env()
    else:
        app.config.from_mapping(test_config)

    db.init_app(app)

    # blueprints, etc.

    return app

[davidism]

There's a few other things that are subtly wrong with your test setup. You call test_request_context first, then call config.update. But test_request_context uses config, what if you wanted to test a different SERVER_NAME, for example? You always need to do things in the order create app, set config, use app. Also, you're unconditionally pushing a request context, which means that the app context will never get torn down between requests, which will cause issues.

[davidism]

The problem is that Flask-SQLAlchemy allows configuring the entire set of engine options, not just a name and URL. Once engines are created they're part of SQLAlchemy, not directly configurable. Determining if any engine configuration has changed is not trivial. It would have to be checked on each access to each engine, even though this would be unnecessary overhead for every request except in tests that change engine config.

This is a problem for any extension that manages third-party objects that live longer than the app/request context, not just Flask-SQLAlchemy. It's a primary issue that using an app factory solves.

[greyli]

The app factory does solve the problem. But in this way, we force users to use the factory to create an app. What if they just need to create a simple project with only a single module app.py, and without involving app factory, current_app, and blueprint?

Whether or not this issue can be resolved, I think it's necessary to add a tip in the docs to indicate the engine-related configuration is not rewriteable (#1175). And I'll update my Flask tutorial/book and introduce the app factory before the test chapter.

Thanks for pointing out the errors in my test setup. I pushed the request context because I want to use url_for in test cases, but I'm not aware of the app context tear-down issue.

[davidism]

It might be possible to allow init_app to be called again on the same app, to update engines based on new config. I'm not enthusiastic about introducing that pattern though, not all extensions can guarantee that calling init_app twice could work correctly.

Perhaps a recreate_engines method? But I don't particularly like that either, because it could just as easily be called from a view function, and I explicitly want to make the antipattern of updating config and changing db in views impossible.

[greyli]

I would go with the recreate_engines method. Is it possible to just put the logic into _call_for_binds (which will be called by create_all, drop_all, and reflect)? In this way, there will be no public method.

I believe we shouldn't limit the way users create the app, especially as an essential Flask extension.

[joeblackwaslike]

@greyli Checkout the docs for Flask extension development for the following guideline

Configuration should not be changed after the application setup phase is complete and the server begins handling requests. Configuration is global, any changes to it are not guaranteed to be visible to other workers.

You're not expected to simply know this, unless you've read the section on flask extension development, which I think is a source of confusion among developers who just use flask extensions. Ignore the guideline at your own peril, as many Flask extensions are predicated on this kind of logic and the results will lack a certain determinism when you try to circumvent this rule.

As someone who likes to break rules I experimented with this idea and there was nothing fruitful to come of it. The main problem is that unless you make the assumption that an app context will always be available and app config values are consistently looked up from current_app.config only across all extensions and your app, you're going to see behavior that is not deterministic manifesting as flakiness in your testing suite.

What I do is this. I use an app factory, that app factory takes a list of extensions to initialize (call init_app) on before returning. I instantiate the app in init.py and again in a pytest app fixture that also sets app.config['TESTING'] = True, and patches the app object in init.py with the one from the test fixture. Failure to do the last part will eventually sneak up on you as flakiness due to having multiple apps with different configurations. Again, you have to use current_app everywhere for app and config lookups, and can't manually import the app object from init.py as it will be out of scope of the patch operation in the test fixture.

[cancan101]

I would also like to see recreate_engines added. We are using pytest-xdist and the worker id gets injected as a fixture. That worker id is then used to resolve the the database url with something like:

@pytest.fixture(scope="session")
def db(worker_id: str) -> Generator[None, None, None]:
    if worker_id != "master":  # pragma: no cover
        db_url_worker = f"{str(settings.DATABASE_URL)}-{worker_id}"
        assert "test" in db_url_worker  # noqa: S101

        # Update the setting for the flask app:
        flask_app.config["SQLALCHEMY_DATABASE_URI"] = db_url_worker
        settings.DATABASE_URL = Secret(db_url_worker)
        # fsa.recreate_engines(flask_app)

For normal, non parallel testing I don't need this craziness as I can se the URL statically in a .env file. It becomes more complicated in the case where at import time I don't know the URLs.

[davidism]

But for that, you should be setting the config before you init the database. That's what the app factory is for, your app fixture would set the config when creating the app, and you'd just use the db object directly.

@pytest.fixture
def app(worker_id):
    ...
    yield create_app({"SQLALCHEMY_DATABASE_URI": db_url_worker})

[cancan101]

The issue is that the I have the app defined at module level:

app = Flask(__name__)
app.config.from_mapping(config)
...
admin = Admin(app, name="MyApp", template_mode="bootstrap3")
....
login_manager = LoginManager(app)

@app.route("/#/")
def login():
   ....

which would require a massive refactoring to move all that to a factory. And even if I could move that to a factory, the issue is I am sub-mounting the WSGI flask app under an ASGI app and the tests are running against the ASGI app so I actually don't even have an app fixture nor could I directly use it.

[davidism]

Yes, that is the issue, you should use an app factory since you want to have tests with different config. And you could still use a factory, you would have a factory for the top-level app that also configured the mounted app.

[cancan101]

Right but even with an app factory, I would still have a problem since the ASGI app is not created through a factory and even if it were, it isn't clear how I would (easily) inject the new WSGI app into it.

[davidism]

I mean, I can't help you with refactoring your specific case here, but I guarantee you it's possible to refactor your code to use a factory for what you need.