-
Notifications
You must be signed in to change notification settings - Fork 9
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
object_storage: config models improvements #136
Conversation
64ac6fd
to
b562e7a
Compare
fa0c76e
to
c4f84ef
Compare
Note that this class is not the same as Python 3.11 `StrEnum`, it is much more basic, but it suits our needs.
This will allow to use object storage configs in nested pydantic models downstream in Discriminated Unions, see: https://docs.pydantic.dev/1.10/usage/types/#discriminated-unions-aka-tagged-unions
The aim is to prevent credential leaks.
This field is present in transfer class (it is optional) - everything will work.
For some object storage operations bucket name is not required (for listing bucket for example).
c4f84ef
to
0e8c61e
Compare
|
||
|
||
class LocalObjectStorageConfig(StorageModel): | ||
directory: str | ||
directory: DirectoryPath |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In theory this may break something, as object is no longer string, but I hope not (and local target should be really primarily used for testing in any case).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It did break things :(
It became impossible to create that model without the validation failing because the folder does not exist at the time the configuration is created. That breaks tests and will break use cases where the configuration is prepared and (de)serialized in a different environment (chroot, container, server, etc...) or just before that directory is created.
I'll prepare a revert PR.
[edit] #140
About this change - What it does
StorageDriver
enumWhy this way
To allow downstream use rohmu configs instead of code duplication. See commit messages for more info.