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

Use default value when settings is blank #729

Merged
merged 2 commits into from
Mar 30, 2022
Merged

Use default value when settings is blank #729

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5520275
Use default value when settings is blank
jyejare Mar 29, 2022
4a381da
Merge branch 'master' into default_when_none
rochacbruno Mar 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion dynaconf/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ def values(self):
def setdefault(self, item, default):
"""Returns value if exists or set it as the given default"""
value = self.get(item, empty)
if value is empty and default is not empty:
if (not value or value is empty) and default is not empty:
Copy link

Choose a reason for hiding this comment

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

@rochacbruno @jyejare It looks like this has broken integration with environment variables.

If I have dynaconf 3.1.8 installed, and the combination of:

  1. settings.yaml field key (nested one level from top map) is present, value is empty
  2. validator with default=None for given field key
  3. envvar set for nested field key (DYNACONF_something__field_key)
  4. settings object configuration that looks like: https://github.com/SatelliteQE/robottelo/blob/master/robottelo/config/__init__.py#L18

My envvar value is no longer set in the resulting box instance, it takes the validator default of None.

I confirmed its this line in particular by modifying the file in site-packages with 3.1.8 installed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mshriver @rochacbruno :

  1. The dynaconf should have the unit test to test this or env var should be prioritise over default if its None.
  2. I wonder why None explicitly being set as a settings default, it should not be as by default it should be Empty from dynaconf.
  3. This could be fixed easily by updating the condition to:
    if (not value or value is empty) and default not in [empty, None]:

Copy link
Member

Choose a reason for hiding this comment

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

@jyejare can you add these notes to #741 and then if possible open the PR + adding the needed unit tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mshriver @rochacbruno Tried reproducing locally and it's not reproducible with 3.1.8! Commented my observations on issue !

self.set(
item,
default,
Expand Down