Conversation
|
@fviard please check |
fviard
left a comment
fviard
left a comment
There was a problem hiding this comment.
Thank you very much for your PR that is quite good and would solve an issue that is encountered by quite a few users and sorry for the long delay for my feedback.
If it would still be possible for you, there is a little change of behavior for this feature that I would prefer:
- Currently, for the s3cfg file, it will read one section or another (so either the default or the profile one).
- I think that it would be better to always try to load the "default" section, and to then optionally load the "profile" on top of that.
- It would probably be easy to do a first version with a slight modification of your code.
Something like creating the ConfigParser with the "[default]" sections, and then if there is the profile, use the "parse_file(...)" function of this cp instance with the "profile" as [sections].
I think that it would be better because if you have to copy paste all the common options in all your profiles, there is not that much any interest compared to use multiple config files. But with this, you could have a common (default) config and just put the specific parts inside each profile.
I added 2 nitpicks directly in review comments.
S3/Config.py
Outdated
| section = is_section.groups()[0] | ||
| in_our_section = (section in sections) or (len(sections) == 0) | ||
| current_section = is_section.groups()[0] | ||
| in_our_section = (current_section in sections) or (len(sections) == 0) |
There was a problem hiding this comment.
I think that there is legacy code, but except if I'm missing something, you can probably do directly:
or not sections:
No need for the len.
| def __init__(self, configfile = None, access_key=None, secret_key=None, access_token=None): | ||
| def __init__(self, configfile = None, access_key=None, secret_key=None, access_token=None, profile=None): | ||
| if configfile: | ||
| debug("Config: Initializing with config file '%s', profile '%s'" % (configfile, profile)) |
There was a problem hiding this comment.
Except this one, you can probably get ride of the other debug logs that I don't think are necessary once you will have a working code.
There was a problem hiding this comment.
Agree, removed all other newly created debug lines.
|
@fviard I have updated and tested the code to always load default. |
4 participants
Allow having few profiles in default config .s3cfg