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

[bug] Default value on empty string #993

Open
LukeSavefrogs opened this issue Sep 4, 2023 · 9 comments
Open

[bug] Default value on empty string #993

LukeSavefrogs opened this issue Sep 4, 2023 · 9 comments
Labels
Milestone

Comments

@LukeSavefrogs
Copy link

LukeSavefrogs commented Sep 4, 2023

Problem

I have a nested structure whose value i need to set to a specific string when empy string or None is provided.

Take the following for example:

from dynaconf import Dynaconf, Validator

settings = Dynaconf(
    settings_files=[
        'config.toml',
        '.secrets.toml'
    ],
    merge_enabled=True,         # Merge all found files into one configuration.
    validators=[                # Custom validators.
        Validator(
            "files.output.kml",
            default="output.kml",
            apply_default_on_none=True,
        ),
    ],
    environments=False,          # Disable environments support.
    apply_default_on_none=True   # Apply default values when a value is None.
)
print(f"KML FILE: '{settings.files.output.kml}'")

With the following configuration file (saved as config.toml):

[files.output]
kml = ""
  • When the kml key is not present in the config file, the default is given to the setting as espected
  • When the kml key is set to an empty string, the default is completely ignored, even if I passed apply_default_on_none=True, while I would expect it to print output.kml

It is somehow related to #973, since if that issue was solved I could simply put a condition=lambda value: value is not None and value.strip() != "" parameter to the Validator and then use the default value since a ValidationError would occur.

What I expected

From the documentation:

Warning

YAML reads empty keys as None and in that case defaults are not applied, if you want to change it set apply_default_on_none=True either globally to Dynaconf class or individually on a Validator.

Reading this I expected the default value to kick in even on empty strings if i set apply_default_on_none=True on the Validator or on the Dynaconf class (I tried both but got the same result).

Workaround

To work around this issue I had to check manually if the setting was still empty:

from dynaconf import Dynaconf, Validator

settings = Dynaconf(
    settings_files=[
        'config.toml',
        '.secrets.toml'
    ],
    merge_enabled=True,         # Merge all found files into one configuration.
    validators=[                # Custom validators.
        Validator(
            "files.output.kml",
            default="output.kml",
            apply_default_on_none=True,
        ),
    ],
    environments=False,          # Disable environments support.
    apply_default_on_none=True   # Apply default values when a value is None.
)

# Setup default values for missing settings
if str(settings.files.output.kml.strip()) == "":
    settings.files.output.kml = "output.kml"

print(f"KML FILE: '{settings.files.output.kml}'")   # Correctly prints `KML FILE: 'output.kml'`

And the same for all the other keys I have to make sure exist.

@pedro-psb
Copy link
Member

@LukeSavefrogs I'm not entirely sure if this is expected, because casting empty string to None is a specific behavior of the YAML parser.

But as you pointed out, this would probably be covered if the ideas in #973 are carried on, which would be great. There is an undergoing refactor on validation implementation of the default setting, so we should await for that, anyway.

In the meantime, I've found a small hack that can be used as a workaround as well. You can use when + cast to set a default when the key is "":

Dynaconf(
        ...,
        Validator(
            "files.output.kml",
            when=Validator("files.output.kml", eq=""),
            cast=lambda value: "output.kml", # hack to use default when str is empty
            default="output.kml", # applies when the value was not set at all
        )
)

@pedro-psb pedro-psb added this to the 3.3.0 milestone Sep 4, 2023
@LukeSavefrogs
Copy link
Author

LukeSavefrogs commented Sep 4, 2023

Thank you @pedro-psb!

In the meantime, I've found a small hack that can be used as a workaround as well. You can use when + cast to set a default when the key is "":

I confirm that indeed the proposed hack works:

  • No key: KML FILE: 'D:\Progetti\danea-automation\src\veryeasyfatt\output.kml'
  • Empty key: KML FILE: 'D:\Progetti\danea-automation\src\veryeasyfatt\output.kml'
  • Key with value (kml = "test"): KML FILE: 'test'

To be able to use cast as usual (in this case to cast the value to a Path object) i slightly changed your code:

Dynaconf(
        ...,
        Validator(
            "files.output.kml",
            when=Validator("files.output.kml", eq=""),
            cast=lambda value: Path("output.kml") if str(value).strip() == "" else Path(value),
            default="output.kml", # applies when the value was not set at all
        )
)

This way, even if the key is not defined, cast gets the default value as a parameter, in the end converting it to Path.

@rochacbruno
Copy link
Member

We must add the workaround example as a test_functional so we ensure it keeps working.

@LukeSavefrogs
Copy link
Author

LukeSavefrogs commented Sep 6, 2023

Sorry I'm a bit confused, shouldn't the actual solution to use the apply_default_on_none as told in the documentation?

If so, this is a bug right?

@pedro-psb
Copy link
Member

YAML reads empty values as None in this case if you want to have Validator defaults applied to the None values you must set apply_default_on_none to True on Dynaconf class or specific on Validator instance.

@LukeSavefrogs Empty values in this case are key-value pairs with no pairs in yaml: key: (no value after), and it is not the same as empty strings. (e.g. TOML parser doesnt even allow empty values, in this sense).

When this happens, YAML parser assign a None value to key. The way I interpret this is that it should apply default when a strict None value is used, and "" is not None.

@rochacbruno rochacbruno modified the milestones: 3.3.0, backlog Sep 7, 2023
@pedro-psb pedro-psb removed the bug label Sep 21, 2023
@pedro-psb
Copy link
Member

I've removed this as a bug because of what I've explained in the previous post about apply_default_on_none behavior.
So we should:

  • add a functional test to ensure the workaround keeps working
  • improve the meaning of apply_default_on_none in the documentation.

Also, one possible way to improve user experience in this use case is to add an "interpret-empty-str-as-none" option. I personally don't like it, as we already have the @empty token which translates to None, but it's a possibility.

@LukeSavefrogs
Copy link
Author

LukeSavefrogs commented Sep 22, 2023

Also, one possible way to improve user experience in this use case is to add an "interpret-empty-str-as-none" option. I personally don't like it, as we already have the @empty token which translates to None, but it's a possibility.

Instead of a new flag which could be very confusing how about a transform function (or list) which runs before any validation occurs and changes the value as per user?
This way one could write a simple lambda lambda s: s if s.strip() != "" else None and pass it to the transform parameter so that every key is converted

What do you think?

@pedro-psb
Copy link
Member

It's an interesting idea to have a more broad custom parsing step. I see the general use cases where you can't directly instruct the person writing the conf files about all dynaconf-specific idiosyncrasies and want to use a custom interpretation layer in between to match their specific needs.

which runs before any validation occurs and changes the value as per user

But "before" should really be completely out of the validation (just enforcing, not sure if you didn't already mean that). We are currently trying to remove any side-effect from the validation process in order to make the codebase more decoupled and maintainable, as validation side-effects have been a particularly noticeable source of bugs. Conceptually it's purpose is to validate a fixed state, so calls to it shouldn't change state.

I believe this should be placed the nearest possible from parsers, like an adapter layer for the user, which would be mostly his responsibility. And it should make it easier for him to debug and reason about later. We already have a basic hooking system, so this could be like a pos_parsing_hook or similar.

@pedro-psb
Copy link
Member

Another possible solution would be to add an access hook to the other end of the flow, on the access layer. When accessing any given key, we would run registered hooks that can implement this fallback logic on empty string values.

This fallback logic on access is related to what we are trying to do on #988. It's easier to implement when using settings.get("a.b.c"), but more tricky with settings.a.b.c.

Also, it may be helpful to have a look at #975 for hooking. Although it was implemented for a specific django use-case, we could try generalizing it.

@pedro-psb pedro-psb modified the milestones: backlog, Backlog Nov 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants