-
Notifications
You must be signed in to change notification settings - Fork 4
feat(server): replace waitress by gunicorn #23
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,4 +20,4 @@ WORKDIR /app | |
|
|
||
| RUN make install | ||
|
|
||
| ENTRYPOINT exec python -m dapytains.app.app | ||
| ENTRYPOINT exec make start | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| import os | ||
| from dapytains.app.app import app, db | ||
| from dapytains.app.ingest import store_catalog | ||
| from dapytains.metadata.xml_parser import parse | ||
| from dotenv_flow import dotenv_flow | ||
|
|
||
| dotenv_flow(os.getenv("SERVER_ENV", "prod")) | ||
|
|
||
| if __name__ == "__main__": | ||
| app.config['SQLALCHEMY_DATABASE_URI'] = os.getenv("DATABASE_URI", "sqlite:///../app.db") | ||
| app.config['SQLALCHEMY_TRACK_MODIFICATIONS'] = False | ||
|
|
||
| db.init_app(app) | ||
| with app.app_context(): | ||
| db.drop_all() | ||
| db.create_all() | ||
|
Comment on lines
+15
to
+16
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i may be wrong here , but wouldn't it be better to conditionnaly control whenever to drop all and recreate the db or not with a flag/arugment
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I completely agree with that comment, but that's not really the purpose of this pull request. This logic should probably be extracted into a command, but I would prefer that to be done in another pull request. |
||
|
|
||
| catalog, _ = parse(os.getenv("DTSCATALOG", "tests/catalog/example-collection.xml")) | ||
| store_catalog(catalog) | ||
|
|
||
| app.run(debug=("prod" != os.getenv("SERVER_ENV", "prod")), host=os.getenv("SERVER_HOST", "0.0.0.0"), port=os.getenv("SERVER_PORT", 5000)) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,4 +6,4 @@ click | |
| uritemplate | ||
| tqdm | ||
| dotenv_flow | ||
| waitress | ||
| gunicorn | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
on this point , i want to note that i don't think it's an urgent change at this stage
also i haven't personnaly used make as entrypoints , but what i'm reading is that it hides the gunicorn process behind make , and for prod it can be problematic for signals in general and shutdowns
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
imho there is no problem; make handles signals very well. The goal here is to start the server using the dotenv variables. Without make we would have to create a start script that would duplicate the logic already present in the makefile.