Closed
Bug 779294
Opened 12 years ago
Closed 11 years ago
BaseConfig.py should allow for multiple config files
Categories
(Release Engineering :: General, defect, P4)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: mozilla, Assigned: mozilla)
References
Details
(Whiteboard: [mozharness])
Attachments
(1 file, 1 obsolete file)
17.17 KB,
patch
|
rail
:
review+
mozilla
:
checked-in+
|
Details | Diff | Splinter Review |
This would change --config-file to an extend or append here: http://hg.mozilla.org/build/mozharness/file/e93ae357f521/mozharness/base/config.py#l197 Also, here we'd need to parse the config files as a list, in order: http://hg.mozilla.org/build/mozharness/file/e93ae357f521/mozharness/base/config.py#l329 This adds enough complexity that it would be good to have unit tests written to test this. While we're at it, maybe --config-file/--cfg is such a high-usage option that it makes a sense to create a single-char option for it like -c or -f.
Assignee | ||
Updated•12 years ago
|
Priority: -- → P4
Comment 1•12 years ago
|
||
-c is fairly common for specifying configuration files. I'm not sure if the leftover args are used at all in any existing mozharness script. If not, those are options too.
Assignee | ||
Comment 2•11 years ago
|
||
Bonus points: * allow for optional config files. right now if the config file doesn't exist, we'll hit an exception. maybe -C required_config_file -c optional_config_file ? I don't know how to preserve order here in OptParse if we do this, however. * load configs from a url. If we solve this bug without one or both of these, we can file new bugs for them.
Assignee | ||
Comment 3•11 years ago
|
||
I'd like some tests around multiple config files.
Assignee: nobody → aki
Assignee | ||
Comment 4•11 years ago
|
||
(In reply to Jeff Hammel [:jhammel] from comment #1) > I'm not sure if the > leftover args are used at all in any existing mozharness script. If not, > those are options too. Not sure if I want the leftover args to be config files or actions.
Assignee | ||
Comment 5•11 years ago
|
||
Looks like this works. * pep8 for mozharness.base.config * a couple new test config files * removed the unused --action and --only-ACTION options (now just use --ACTION or --no-ACTION or --add-action ACTION) * tests for multi config files
Attachment #720225 -
Flags: review?(rail)
Comment 6•11 years ago
|
||
Comment on attachment 720225 [details] [diff] [review] with tests, fix Review of attachment 720225 [details] [diff] [review]: ----------------------------------------------------------------- I was a little bit confused by the "extend" action of ExtendOption, because I was expecting "append" instead (as list's methods). Then I read the source and found that it splits parameters by coma, so accepts multiple parameters, what makes sense.
Attachment #720225 -
Flags: review?(rail) → review+
Assignee | ||
Comment 7•11 years ago
|
||
Yeah, sorry for not mentioning that. Thanks for the review!
Assignee | ||
Updated•11 years ago
|
Attachment #712825 -
Attachment is obsolete: true
Assignee | ||
Comment 8•11 years ago
|
||
Comment on attachment 720225 [details] [diff] [review] with tests, fix http://hg.mozilla.org/build/mozharness/rev/56f801571d30
Attachment #720225 -
Flags: checked-in+
Assignee | ||
Comment 9•11 years ago
|
||
I'd like to see some Cedar green with this on default before merging to production. Filed bug 847526 for comment 2.
Assignee | ||
Comment 10•11 years ago
|
||
Merged to production after seeing a green Cedar test.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Product: mozilla.org → Release Engineering
Updated•6 years ago
|
Component: General Automation → General
You need to log in
before you can comment on or make changes to this bug.
Description
•