Closed Bug 1007292 Opened 10 years ago Closed 6 years ago

for Mozharness, hide Buildbot integration

Categories

(Release Engineering :: Applications: MozharnessCore, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: jlund, Unassigned, Mentored)

References

Details

(Whiteboard: [easier-mozharness][good next bug])

Attachments

(1 file)

This will require two things:

1) deal with buildbot_properties and buildbot_config
2) deal with buildbot status

With the help from 1007280, we can hide (1) so that it is only used in automation, and ignored outside of it.

Patch for (1) incoming
requires nosetests before moving forward

There is probably an easier way to share the log object than importing __future__ modules and passing log methods via args (logger)
As a slightly different option it may make more sense to:

let's call this OPTION B: we could treat self.buildbot_config (buildprops.json) not as self.live_config items, but as self.config items since they are known before the script runs (goes live). So we would do something like:

if automation:
    `./scripts/desktop_unittest.py --cfg unittests/linux_unittest.py --cfg buildprops.json --mochitest-suite plain1`
if developer:
    `./scripts/desktop_unittest.py --cfg unittests/linux_unittest.py --cfg unittests/developer_unittest.py --mochitest-suite plain1`

where developer_unittest.py consists of a template that devs can use to fill out missing info: https://pastebin.mozilla.org/5095328

arguments against this:
   - possibly will be hard to maintain a developer config.
       * my response would be I don't see it requiring too many items
   - it's too many options for devs
       * my response would be I think we could have defaults here and if they can't have defaults than devs would have to fill it out anyways.


Having one dev template config per mozharn script, IMO, is easier to deal with than branching mozharness or doing a major mozharness overhaul(functions and globals style).

in this OPTION B scenario, self.buildbot_properties would still be replaced by self.live_config as it accurately represents config items that are collected live as the script runs through actions. And self.live_config still makes a cache of itself so actions have a chance to be independent (when we don't clobber)

 Basically, anytime we inherit BuildbotMixin, this would allow us to:

    - remove 'read_buildbot_config' from every script actions list (it would be done only when we are within automation during pre and post run hook methods)
    - OPTION B changes: anytime we use `self.buildbot_config.get()`, we instead would use `self.config.get()` like we do for everything else and is easier to grep for new comers faced with multiple config objects.
    - anytime we use `self.set_buildbot_property(prop, val)`, we instead would use `self.live_config['prop'] = val`
also, in other situations where we use things like tree configs, I think these could be part of self.config.

I see self.config as the source for truth that knows everything that it can know about a specific script run before that script starts (and nothing more). 

so instead of having things like:

self.buildbot_config
self.tree_config
self.buildbot_properties

we have:

self.config
self.live_config

IIUC, we can populate self.config with what would be self.tree_config, by passing the tree specific config as a URL to --cfg when calling the script.


Finally,

I still like the idea of having more 'helper objects' like vcs objects their own configs and letting them do things like clone in parallel. But I think the helper object's configs can be populated via items in self.config or during runtime decisions (self.live_config).

sorry to rant. Just wanted to get thoughts out so they can be discussed. hope some of it makes sense :)
Component: General Automation → Mozharness
Mentor: jlund
Whiteboard: [easier-mozharness]
See Also: → 978865
Whiteboard: [easier-mozharness] → [easier-mozharness][good next bug]
buildbot is dead/dying!
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: