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

implement configurable factory #28

Merged
merged 2 commits into from
Dec 14, 2022
Merged

implement configurable factory #28

merged 2 commits into from
Dec 14, 2022

Conversation

briantist
Copy link
Owner

This PR adds a new factory method create_configured_app, and pushes all of the "main" CLI code into that method.

The idea is to use the CLI (and especially config and environment) parsing to set the options so that they don't all have to be set explicitly as they do in the bare create_app method.

This should be helpful for using galactory with a production WSGI server.

For example, with this change, you could run gunicorn -w 4 'galactory:create_configured_app()' and it would take values from environment variables and your config file(s).

I haven't tried it with other WSGI servers yet, but at least with gunicorn it also works to specify additional CLI arguments, for example: gunicorn -w 4 'galactory:create_configured_app()' --log-level DEBUG

The existing method of running with the built-in werkzeug server is unchanged and still works.

@briantist briantist added the enhancement New feature or request label Dec 14, 2022
@briantist briantist self-assigned this Dec 14, 2022
@codecov
Copy link

codecov bot commented Dec 14, 2022

Codecov Report

Base: 53.31% // Head: 51.59% // Decreases project coverage by -1.71% ⚠️

Coverage data is based on head (52d6bbb) compared to base (2b898d2).
Patch coverage: 20.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #28      +/-   ##
==========================================
- Coverage   53.31%   51.59%   -1.72%     
==========================================
  Files          19       19              
  Lines         769      814      +45     
==========================================
+ Hits          410      420      +10     
- Misses        359      394      +35     
Impacted Files Coverage Δ
galactory/__init__.py 42.64% <20.00%> (-44.31%) ⬇️
galactory/utilities.py 42.44% <0.00%> (+0.71%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@briantist briantist merged commit d0e595f into main Dec 14, 2022
@briantist briantist deleted the factory-retrofits branch December 14, 2022 02:47
@jcox10
Copy link
Contributor

jcox10 commented Dec 19, 2022

Would it be worth updating the Docker image to use gunicorn?

@briantist
Copy link
Owner Author

@jcox10

Would it be worth updating the Docker image to use gunicorn?

I am considering it, or maybe adding additional images with various other servers. I am debating whether it's better to do that, or let folks build their own image anyway (where it would be easier to embed a config file and such), and I'm not sure how many different images I want to support directly.

@jcox10
Copy link
Contributor

jcox10 commented Dec 23, 2022

I would just maintain a single image that is good enough to run in a development/test environment and serves as an example for more advanced use cases. If people run behind a reverse proxy, then gunicorn probably doesn't do much; however, if they start hitting the container directly then it might be beneficial.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants