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.

Sign up for GitHub

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

Jump to bottom

Web API PRODUCTION environment variable is unnecessarily obtuse #146

Closed
dhdaines opened this issue Feb 6, 2023 · 0 comments · Fixed by #147
Closed

Web API PRODUCTION environment variable is unnecessarily obtuse #146

dhdaines opened this issue Feb 6, 2023 · 0 comments · Fixed by #147

Comments

@dhdaines
Copy link
Collaborator

dhdaines commented Feb 6, 2023 •
edited
Loading

The way of telling the web API to go into development mode makes no sense:

PRODUCTION= uvicorn readalongs.web_api:web_api_app --reload

If production mode is the default then we should instead have a DEVELOPMENT environment variable that enables permissive CORS headers.

The PRODUCTION variable means nothing to uvicorn and it means nothing to FastAPI, we are the only ones using it. Unless there is some dark Heroku magic involved? But even then, why would it be production mode if (a) PRODUCTION is unset, or if (b) PRODUCTION is set to something truthy?
dhdaines added a commit that referenced this issue Feb 6, 2023
@dhdaines
feat: introduce better CORS environment variables
1eb5d74 
instead of the confusing PRODUCTION= you now do DEVELOPMENT=1 (or
anything else truthy), and you can also explicitly specify the
deployed origin URL with ORIGIN if you like for production mode
(fixes #146)
dhdaines added a commit that referenced this issue Feb 6, 2023
@dhdaines
feat: introduce better CORS environment variables
d8ab4cf 
instead of the confusing PRODUCTION= you now do DEVELOPMENT=1 (or
anything else truthy), and you can also explicitly specify the
deployed origin URL with ORIGIN if you like for production mode
(fixes #146)
@dhdaines dhdaines mentioned this issue Feb 6, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Introduce better CORS environment variables ReadAlongs/Studio
1 participant
@dhdaines