Closed
Bug 725097
Opened 13 years ago
Closed 13 years ago
remove .config files and transition these to classes
Categories
(Testing :: Talos, enhancement)
Testing
Talos
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: k0scist, Assigned: k0scist)
References
Details
To build a yml file for run_tests.py, we currently read in a .config file:
http://hg.mozilla.org/build/talos/file/8263f6cc9ec0/talos/PerfConfigurator.py#l207
then we substitute for each line, usually checking for a 'key:' in the line and doing a lot of one-off magic:
http://hg.mozilla.org/build/talos/file/8263f6cc9ec0/talos/PerfConfigurator.py#l90
This was *probably*(?) done so that the resulting yml files could have the same order and the same comments as the .config seed file. This is good.
However, there are several unintended bad things that result from this:
- having to add things multiple places: currently if you want a new key, value (or worse) to all of the configuration files, you have to add it in 6 places, one per config file. This is error prone let alone annoying.
- the reconstruction is error prone. For example, http://hg.mozilla.org/build/talos/file/tip/talos/PerfConfigurator.py#l186 relies on 'testbranch' not being in the line. While this is not very likely, it is entirely possible that someone could mess up the parsing by having an unintended key conflict. Generally, serialization is done by, um, serializing a set of data or interpolating a template, not trying to patch based on a set of magic keys.
- This isn't very programmatic. If we were straight serializating to YAML, we could presumedly add a key and a default value to a Configuration class and have it basically work. Instead, there are many different things to fix many different places for each option we add.
Instead I would propose making the different .configs into python classes. This way, parsing and serializing is straight-forward. If you use an inheritence model, you should be able to add a single key, value (etc) to the base class and have the concrete classes automagically use it. If we care about comments and the correct order of things, etc, one better option to serialization is templates, which could be as simple as, e.g.
"""
# the foo option means something
foo: %(foo)s
""" % dict(foo=foo)
There is also string.Template
Longer term, each config file, which will now be a class, will replace what PerfConfigurator.PerfConfigurator is now. That is, the base class (i.e. Configuration, not to be confused with the badly named Exception of the same name) will have all the logic that is currently in PerfConfigurator.PerfConfigurator. Most of the other classes won't need any logic, except remote.config <=> remotePerfConfigurator.py . But even without this, it would be a big boon to have configuration as classes vs what we have now.
This is a big blocker for bug 704654
Assignee | ||
Comment 1•13 years ago
|
||
I plan to do a pretty straight refactor so some parts of files may remain for overrides, but most configuration should be determinable from the command line and a programmatic seed
Assignee: nobody → jhammel
Assignee | ||
Comment 2•13 years ago
|
||
fixed with bug 704654
There are a few .config files left with overrides, and sample.config and remote.config remain though they are blank files (remote.config is assuredly used for automation and I was afraid sample.config might be) but these can be deprecated with buildbot changes
Anyway, there may be some cleanup for this in general but I will close and let people post follow-up bugs
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•