acederberg / pydantic-settings-yaml

A convenient tool for loading pydantic settings from either YAML and JSON.
MIT License
5 stars 3 forks source link

arbitrary file load order #23

Open mikeshaw57 opened 3 months ago

mikeshaw57 commented 3 months ago

I'm able to load multiple files and inject ENV vars and secret files, excellent!

However, I'm trying to also use it to override settings and it is not working consistently. I.e.

model_config = YamlSettingsConfigDict(
    ...
    yaml_files= [
        f"{__conf_path__}/settings.yaml",
        f"{__conf_path__}/settings.{os.getenv('CFG_TARGET_ENV', 'dev')}.yaml"
    ]
)

loading all settings from settings.yaml then overriding values as provided in settings.dev.yaml, settings.test.yaml, etc

It seems like the files are deserialized in an arbitrary order. Should I be able to do this with this module or not a planned use case?

mikeshaw57 commented 3 months ago

I see, "Bulk load files" for load performance. makes sense. I'll just avoid duplicate keys across main/override files

acederberg commented 3 months ago

Thank you for using this library @mikeshaw57!

I am pleased to hear that it has been helpful. The above example will result in settings.yaml and then settings.dev.yaml being loaded and merged (where the values of the latter take precedence). These settings are put into Pydantic's deep_update utility function in the order that the are specified in yaml_files.

Note about yaml_files

Your observation is true in an edge case though. For instance in the case where yaml_files is a Set. It looks like there is an issue with the current type hint for the yaml_files field:

yaml_files: Set[str] | Sequence[str] | Dict[str, YamlFileConfigDict] | str

since Set is not ordered (it is not a Sequence). However, I've used this exact functionality in some cases where the files don't intersect in their content and everything has been okay.

mikeshaw57 commented 3 months ago

thanks, ace are you saying that this should load in order? as you indicated the "above example"

model_config = YamlSettingsConfigDict(
    ...
    yaml_files= [
        f"{__conf_path__}/settings.yaml",
        f"{__conf_path__}/settings.{os.getenv('CFG_TARGET_ENV', 'dev')}.yaml"
    ]
)

Array (Sequence) literal, not Set literal. It merges, but if I have intersecting keys between the 2 files than there's an issue as it does not apply in the order specified by the array literal. I'm fine with this circumstance, but if I can have intersecting, it would be nicer.

Apologies, I'm from C# and Java background and not fully intimate with python quite yet

PS - thanks for the handy extension!

acederberg commented 3 months ago

Hi @mikeshaw57,

In this case it appears you have found a bug - what is happening (arbitrary load order) should only really happen in the case of set literals but not with sequences. It appears there are no tests for this so tests must be added to address this.

I'm currently wrapped up in some other stuff but this should be addressed at some point in the next week or so. I'll roll this in to fixing #22.s

No need to apologize, you could have fooled me haha

mikeshaw57 commented 3 months ago

@acederberg - Ordering with intersection tolerance would be nice, but not a show-stopper. I'm just avoiding intersection for now.