Closed Bug 725097 Opened 13 years ago Closed 13 years ago

remove .config files and transition these to classes

Categories

(Testing :: Talos, enhancement)

enhancement
Not set
normal

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
Blocks: 725140
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
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.