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

Strict mode for top non-default keys #6

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

akscram
Copy link
Contributor

@akscram akscram commented Dec 24, 2016

No description provided.

return True
elif env_value == "false":
return False
return None
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks strange, how debug could be non-True and non-False in the same time.
Why not simply decide if debug is True or False, right here?

Also, just recommendation: consider checking "0" and "1", "False" and "True" as well.
Another approach is strict check - if env_value is non-empty and neither "true" nor "false" then raise an error with message that env variable is set to strange value

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The debug environment variable can be set and it can have correct value, such as "true" and "false", incorrect value, or can be not set at all. Also, the default value is determined later in the code.

However, I agree that a validation have to be done in this function instead of silently hide all incorrect values and return None as though they are not set. I will fix it in the next patches.

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.

2 participants