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

[omm] deplopulate __init__ and delint #1368

Merged
merged 1 commit into from
Sep 25, 2023
Merged

Conversation

Dcallies
Copy link
Contributor

@Dcallies Dcallies commented Sep 22, 2023

Summary

Depopulating __init__.py

A bunch of the lint tools were behaving strangely because they were getting confused about contents of the project based on the behavior of the __init__.py module. __init__.py is special in that its executed whenever you import subprojects (import a.b executes a.init.py).

Putting the flask application creation factory (create_app) in __init__.py is initially convenient because it means the factory is always at the top level of the init, however it introduces a lot of potential for errors (I've cut myself with __init__.py enough that I now prefer to leave them empty).

How flask finds the right app is described in https://flask.palletsprojects.com/en/2.3.x/cli/#application-discovery

The other common place to put it is in app.py.

As part of this work, I also tried to remove all the module-level work that was being done, but specifically for the "db" object, this doesn't appear to be possible due to the design of flask-sqlalchemy.

I decided to just arbitrary shuffle the database information all into one place (models and all).

Delint

emptying __init__.py allowed me to delint the rest of app, which were mostly small fixes.

Test Plan

  1. Rebuild the docker container from fresh, see that everything boots up correctly.
  2. Manually run startup.sh (needed when you accidentally fatal your original copy), note it runs clean.

mypy . && py.test

LINT SHOULD NOW BE GREEN!

@Dcallies
Copy link
Contributor Author

Request for reviewers is that unless I've made some fatal error in this step, if there are minor changes to be made that I can do them in followups, since I think getting the CI green is useful!

@Dcallies Dcallies merged commit 35a8ed4 into main Sep 25, 2023
5 checks passed
@Dcallies Dcallies deleted the project_relayout_to_fix_lints branch September 25, 2023 13:17
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