-
-
Notifications
You must be signed in to change notification settings - Fork 165
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
fs_storage: add possibility to define options values from env. #303
Conversation
8855421
to
fb43a86
Compare
Looks good (code review). This should simplify configuration by letting us configure everything in a record with embedded environment variables such as, say, |
This PR has the |
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.
LG overall. I'll test later
_logger.warning( | ||
f"Environment variable {env_variable_name} is not set for " | ||
f"fs_storage {self.display_name}." |
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.
_logger.warning( | |
f"Environment variable {env_variable_name} is not set for " | |
f"fs_storage {self.display_name}." | |
_logger.warning( | |
"Environment variable %s is not set for " | |
"fs_storage %s", env_variable_name, self.display_name |
|
||
eval_options_from_env = fields.Boolean( | ||
string="Resolve env vars", | ||
help="Resolve options values starting with $ from environment variables", |
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.
I would provide an example here
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.
Thank you @adrienpeiffer
Can you complete the documentation in https://github.com/OCA/storage/blob/16.0/fs_storage/readme/USAGE.rst to describe this new option for configuring a storage backend?
Can you also add a file 303.feature
into the fs_storage/readme/newsfragment
directory to describe this new feature. This file will be use at merge to fill the history section into the readme file and help users to know what changes have been made between versions.
5c3f9c4
to
f2fe690
Compare
f2fe690
to
8c31c7e
Compare
@lmignon Could be merged for Xmas ? |
/ocabot merge minor |
Hey, thanks for contributing! Proceeding to merge this for you. |
@adrienpeiffer can you handle the two comments from Simone? |
I have cancelled the merge. |
Congratulations, your PR was merged at 359ac46. Thanks a lot for contributing to OCA. ❤️ |
@sbidoul Seems not cancelled. I'll handle that in an other PR. |
No description provided.