-
-
Notifications
You must be signed in to change notification settings - Fork 915
Option to specify profile #1420
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
base: master
Are you sure you want to change the base?
Conversation
|
@fviard please check |
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 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.
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Allow having few profiles in default config .s3cfg