Closed Alyxion closed 1 year ago
Thanks a lot for the PR and sorry for the late review!
Some points from my side:
populat_config
, not handle them separately but pass through the same steps? get_format
could check whether the format is specified if it gets bytes, create_stream
could instantiate the BytesIO
stream and the other functions are called anyways from populate_config_from_bytes
. I would prefer to have it in the same structure or not support bytes at all, instead of having a special case for it.Path
, I think it would be great to do this consistently. So also extend ConfZFileSource.folder
and ConfZEnvSource.file
. Ideally the same range of formats (at least for the files, not for the folder of course).Path
examples in the readme and the documentation, as it reduces clutter (as you wrote in the PR description).Thanks again and let me know your thoughts on that.
_"Should we try to have the same overall control-flow for bytes as for paths/string? So in populat_config, not handle them separately but pass through the same steps? get_format could check whether the format is specified if it gets bytes, create_stream could instantiate the BytesIO stream and the other functions are called anyways from populate_config_frombytes. I would prefer to have it in the same structure or not support bytes at all, instead of having a special case for it."
I tried to minimize the redundancy already as good as I could with my (still quite small) overall oversight of the project :). I would suppose I will fix the the env file and folder definitions and you could then refactor it after afterwards.
"If we start to support other path formats than Path, I think it would be great to do this consistently. So also extend ConfZFileSource.folder and ConfZEnvSource.file. Ideally the same range of formats (at least for the files, not for the folder of course)."
Sure, didn't work with the env variant before so I missed that one, the folder is obvious of course ;).
Regarding the readme and documentation: Of course, I just didn't want to fiddle around in your landing page as the old examples did not become invalid, just may be not the optimal way anymore.
So, sorry for the delay, hopefully everything adjusted now as discussed :), hope I didn't miss something in the documentation. By chance the possibility to pass data directly via bytes should be added there too.
As dotenv supported streams out of the box enhancing the environment path was straight forward.
Look great, thanks a lot!
The configuration source file (ConfZFileSource.file) can now - following the suggestions of PEP 519 - not just be defined by a Path anymore but also as
str
instead of a Path object or even directly as abytes
-string with the content of the configuration file.If the content is provided as bytes string the definition of the format is mandatory as there is no file path anymore from which it could be derived.