mariusmucenicu / knowlift

A software that will keep you sharp between the ears.
https://knowlift.com/
GNU General Public License v3.0
0 stars 1 forks source link

Change assertion error message to better reflect the absence of a db #204

Closed mariusmucenicu closed 5 years ago

mariusmucenicu commented 5 years ago

In order for SQLAlchemy to connect to an SQLite database , a database name is needed. That is, if you want to connect to a persistent database. If, on the other hand, you'll want to create to an in-memory database, you can just leave the database name out and just do engine = create_engine('sqlite://').

This scenario can happen, if for instance we'd like to fetch the database via an environment variable, which is either not set, or overridden later on to be None or ''. Different configs fetch the database name differently, which, in turn, can also be overridden so when initializing the database we must ensure that a name is present.

In the config, especially in the ProductionConfig we do a call of the form DATABASE = os.environ.get('FLASK_DATABASE') which will result in the value None if the database is not present. We could change this to have the default value of empty string, but that would imply changing everywhere else where we fetch the database like this, and the same goes for anywhere in the future where we might fetch a database like this.

None and '' are different when passing the value to create_engine which is why it matters, but since both will fail at the assertion step, indicating that we don't allow the absence of a value semantically they're the same. So it's easier to change the assertion error message to better reflect what's going on.

Also this forbids Flask to start regardless of the Config used if there's no database value. This indicates the fact that in-memory SQLite should be used only interactively (via the CLI) to quickly test something, and not with this application.