Use default value when settings is blank #729
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
3 tasks
rochacbruno
added a commit
that referenced
this pull request
Apr 15, 2022
Shortlog of commits since last release:
Anderson Sousa (1):
Document the usage with python -m (#710)
Andressa Cabistani (2):
Add unique label when merging lists to fix issue #653 (#661)
Add new validation to fix issue #585 (#667)
Armin Berres (1):
Fix typo in error message
Bruno Rocha (7):
Release version 3.1.7
Found this bug that was duplicating the generated envlist (#663)
Add support for Python 3.10 (#665)
Attempt to fix #555 (#669)
Create update_contributors.yml
Fixing pre-coomit and docs CI
Added `dynaconf get` command to cli (#730)
Caneco (2):
improvement: add brand new logo to the project (#686)
improvement: update socialcard to match the python way (#687)
EdwardCuiPeacock (2):
Feature: add @Jinja and @Format casting (#704)
Combo converter doc (#735)
Eitan Mosenkis (1):
Fix FlaskConfig.setdefault (#706)
Enderson Menezes (Mr. Enderson) (2):
Force PYTHONIOENCODING to utf-8 to fix #664 (#672)
edit: move discussions to github tab (#682)
Eugene Triguba (1):
Fix custom prefix link in envvar documentation (#680)
Gibran Herrera (1):
Fix Issue 662 Lazy validation (#675)
Jitendra Yejare (2):
Load vault secrets from environment less stores or which are not written by dynaconf (#725)
Use default value when settings is blank (#729)
Pavel Alimpiev (1):
Update docs link (#678)
Ugo Benassayag (1):
Added validate_only_current_env to validator (issue #734) (#736)
Waylon Walker (1):
Docs Fix Spelling (#696)
dependabot[bot] (3):
Bump django from 2.1.5 to 2.2.26 in /example/django_pytest_pure (#711)
Bump mkdocs from 1.1.2 to 1.2.3 (#715)
Bump django from 2.2.26 to 2.2.27 in /example/django_pytest_pure (#717)
github-actions[bot] (2):
[automated] Update Contributors File (#691)
[automated] Update Contributors File (#732)
lowercase00 (1):
Makes Django/Flask kwargs case insensitive (#721)
mshriver
reviewed
Apr 19, 2022
| @@ -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: | |||
There was a problem hiding this comment.
@rochacbruno @jyejare It looks like this has broken integration with environment variables.
If I have dynaconf 3.1.8 installed, and the combination of:
- settings.yaml field key (nested one level from top map) is present, value is empty
- validator with default=None for given field key
- envvar set for nested field key (DYNACONF_something__field_key)
- 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.
There was a problem hiding this comment.
- The dynaconf should have the unit test to test this or env var should be prioritise over default if its None.
- I wonder why
Noneexplicitly being set as a settings default, it should not be as by default it should be Empty from dynaconf. - This could be fixed easily by updating the condition to:
if (not value or value is empty) and default not in [empty, None]:
There was a problem hiding this comment.
@mshriver @rochacbruno Tried reproducing locally and it's not reproducible with 3.1.8! Commented my observations on issue !
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem Statement:
Currently, when the value for a setting is blank in YAML, the dynaconf settings object is returning the None value instead of what's set as a default parameter.
The Validator example -
Validator('azurerm.client_secret', default='myAzurePass'),is returning None when client_secret is set blank in azure.yamlExpected:
When the Settings is set to blank in the YAML file then it must use the default value.
The Validator example -
Validator('azurerm.client_secret', default='myAzurePass'),should return 'myAzurePass' when client_secret is set blank in azure.yamlSolution:
The
setdefaultmethod in the dynaconf settings class is updated to accept the value as blank to return the value from default parameter.