-
Notifications
You must be signed in to change notification settings - Fork 1
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
adjust service settings #21
Conversation
- filter out invalid Django settings from environment - coerce environment string values to Python literals - rename `merge_dicts` function name and arguments - add docstrings - add doctests to docstrings and default pytest call
if value.lower() == "true": | ||
d[key] = True | ||
elif value.lower() == "false": | ||
d[key] = False | ||
elif value.lower() == "none": | ||
d[key] = None | ||
elif value.isdigit(): | ||
d[key] = int(value) | ||
elif value.replace(".", "", 1).isdigit(): | ||
d[key] = float(value) | ||
else: | ||
d[key] = value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is all good, but if feels like a solved problem vs. using environs
or one of the many libraries that covers all the things. https://github.com/sloria/environs/blob/master/environs/__init__.py#L343C32-L350
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, point taken. I was basically just trying to get it working, even if it's a bit quick and dirty. I'll open an issue to track using environs
or another similar library.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I put a note about if you want to support the env key/value logic.
merge_dicts
function name and arguments