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

Add --app-dir option for running uvicorn from any location #619

Conversation

yezyilomo
Copy link
Contributor

This PR is an attempt to solve #549
I chose --app-dir to be the name of the option, please let me know if it makes sense, I've also updated the documentation to include --app-dir option, please check if I have added it to the right place.

Copy link
Member

@florimondmanca florimondmanca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My two cents on the --app-dir vs --chdir naming…

The gunicorn description of --chdir is:

Chdir to specified directory before apps loading.

This is not accurately what we do here — we don't cd or really change the working directory. Instead, we allow Python to find the APP in the specified directory by adding it to sys.path.

So --app-dir is probably okay IMO?

docs/settings.md Outdated Show resolved Hide resolved
uvicorn/main.py Outdated Show resolved Hide resolved
docs/settings.md Outdated Show resolved Hide resolved
@anein
Copy link

anein commented Jun 8, 2020

Waiting the approval of this meaningful PR. 😭

//cc @florimondmanca

@yezyilomo
Copy link
Contributor Author

Waiting the approval of this meaningful PR. 😭

//cc @florimondmanca

& //cc @tomchristie

docs/deployment.md Outdated Show resolved Hide resolved
@tomchristie
Copy link
Member

Yup, I like this, thanks! One query about the phrasing of the help text... should we maybe prefer "Defaults to the current working directory."?

@yezyilomo yezyilomo requested a review from tomchristie June 9, 2020 09:48
Copy link
Member

@tomchristie tomchristie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fab, thanks!

@tomchristie tomchristie merged commit 77468df into encode:master Jun 9, 2020
@yezyilomo yezyilomo deleted the add_app-dir_option_for_running_uvicorn_from_any_location branch June 9, 2020 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants