847526 - mozharness config file enhancements

Status: RESOLVED FIXED

(Release Engineering :: Applications: MozharnessCore, enhancement, P5)

Description

[Aki Sasaki (not active)]

per bug 779294, comment 2:

* allow specifying required/optional config files from the command line
** (-C / -c ?)
** certainly possible that we read all the required config files, in order, then read any existing optional config files
** this would allow for -c <dir>/<platform>.py (f/e) and only create the platform files that need overriding.

* allow specifying a url instead of a path

Comment 1

[Jason Yeo[:jyeo]]

Attachment: opt_conf.diff

I added the --opt-cfg option to indicate the optional files. The optional config files will override configs specified in --cfg.

I also wrote some tests to make sure I understood the bug correctly. Hope it's okay.

Comment 2

[Jason Yeo[:jyeo]]

(In reply to Aki Sasaki [:aki] from comment #0)
> * allow specifying a url instead of a path

This is fixed in http://hg.mozilla.org/build/mozharness/rev/782f51f796b2.

Comment 3

[Aki Sasaki (not active)]

Comment on attachment 781003 [details] [diff] [review]
opt_conf.diff

>+            # append opt_config to allow them to overwrite previous configs
>+            all_config_files = options.config_files + options.opt_config_files
>+            for cf in all_config_files:
>                 if '://' in cf: # config file is an url
>                     file_name = os.path.basename(cf)
>                     file_path = os.path.join(os.getcwd(), file_name)
>                     download_config_file(cf, file_path)
>                     config.update(parse_config_file(file_path))
>                 else:
>                     config.update(parse_config_file(cf))

The problem with this approach is that the opt_config_files become required at this point.

The

>                     download_config_file(cf, file_path)

and

>                     config.update(parse_config_file(cf))

lines both need to be able to deal with the fact that the url or path might not exist -- if it's not an optional config file, we should die.  If it is an optional config file, at worst we should throw a warning, but continue.

I'm not sure how you want to handle that -- either a flag that we toggle based on whether |cf in options.opt_config_files| or having the for loop call a helper method or what.  You're close, though!

>+    def test_optional_config_files_override_value(self):
>+        c = config.BaseConfig(initial_config_file='test/test.py')
>+        c.parse_args(['--cfg', 'test/test_override.py,test/test_override2.py',
>+                      '--opt-cfg', 'test/test_optional.py'])
>+        self.assertEqual(c._config['opt_override'], "new stuff")
>+
>+    def test_optional_config_files_keep_string(self):
>+        c = config.BaseConfig(initial_config_file='test/test.py')
>+        c.parse_args(['--cfg', 'test/test_override.py,test/test_override2.py',
>+                      '--opt-cfg', 'test/test_optional.py'])
>+        self.assertEqual(c._config['keep_string'], "don't change me")

Thanks for the tests!

Another test, if you feel like adding it, might be specifying an --opt-cfg test/this_file_does_not_exist.py.

Comment 4

[Jason Yeo[:jyeo]]

Attachment: opt_conf.diff

- warn when optional config file not found
- added one more test case

Comment 5

[Jason Yeo[:jyeo]]

Pushed to https://hg.mozilla.org/build/mozharness/rev/2c8033e67f66