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

allow setting run mode via env, add helpers to determine it #4168

Merged
merged 2 commits into from
Oct 21, 2024

Conversation

benedikt-bartscher
Copy link
Contributor

@benedikt-bartscher benedikt-bartscher commented Oct 13, 2024

bonus: error out if backend-only and frontend-only is set at the same time

needed here: reflex-dev/reflex-chakra#22

@benedikt-bartscher benedikt-bartscher marked this pull request as ready for review October 13, 2024 18:31
benedikt-bartscher added a commit to benedikt-bartscher/reflex-chakra that referenced this pull request Oct 13, 2024
Returns:
True if the app is running in frontend-only mode.
"""
return os.environ.get(constants.ENV_FRONTEND_ONLY_ENV_VAR, "").lower() == "true"
Copy link
Collaborator

Choose a reason for hiding this comment

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

For some env vars, we're checking against ["true", "1", "yes"] rather than just "true".

I think it'd be better to have an uniform method doing the check for boolean env var rather than duplicating logic everywhere.

Copy link
Contributor Author

@benedikt-bartscher benedikt-bartscher Oct 14, 2024

Choose a reason for hiding this comment

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

Thanks for your review. I know, it's a mess currently. I think this needs a refactor over the whole repo, maybe we should address that one in a different PR later on.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i created #4172 to discuss/refactor boolean env var handling

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for making that issue.

Yeah I'll approve this PR and we can clean up things over the whole repo in a different PR.

@benedikt-bartscher
Copy link
Contributor Author

needs coordination with #4192

@picklelo picklelo merged commit 7560bf6 into reflex-dev:main Oct 21, 2024
31 checks passed
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.

3 participants