[users at bb.net] Fwd: Re: One anecdote, and some unsolicited code.

[Neil Gilmore]

Ah, geez. Messed up who I was sending to again...


-------- Forwarded Message --------
Subject: 	Re: [users at bb.net] One anecdote, and some unsolicited code.
Date: 	Fri, 18 Nov 2016 10:24:42 -0600
From: 	Neil Gilmore <ngilmore at grammatech.com>
To: 	Pierre Tardy <tardyp at gmail.com>



Good morning, Pierre,

My scheme wouldn't work, as is, anyway.  Someone would inevitably want 
to change the build interval on a Periodic, it wouldn't happen, and my 
inbox would fill up. But it would fix out current major hassle -- not 
being able to modify the builderNames in a scheduler. I would rewrite my 
hack to use comp_attrs as the things to copy over during 
reconfigServiceWithSibling, except...

Too many of the children of BaseScheduler don't appear to have their 
comp_attrs set up properly. This would seem to be in accord with your 
statement that the reconfig API isn't supported in the schedulers yet.

For example (from rc2):

ClusteredBuildbotService:
     compare_attrs = ('name',)

BaseScheduler:
     compare_attrs = ClusteredBuildbotService.compare_attrs + \
         ('builderNames', 'properties', 'codebases')

Timed:
     compare_attrs = ('reason', 'createAbsoluteSourceStamps', 
'onlyIfChanged',
                      'branch', 'fileIsImportant', 'change_filter', 
'onlyImporta\
nt')

ForceScheduler:
     compare_attrs = base.BaseScheduler.compare_attrs + \
         ('builderNames',
          'reason', 'username',
          'forcedProperties')

Thus, Timed does not consider name, builderNames, properties, or 
codebases when assessing 'sameness'. This is clearly incorrect. On the 
other hand, ForceScheduler has builderNames twice, once from 
BaseScheduler and again from itself. Also incorrect, but probably benign.

This is correctable, and if I understand, necessary to ensure that the 
whole mechanism works correctly. If the SchedulerManager can't correctly 
figure out the 'sameness' of schedulers, the whole thing falls apart.

It's also correctable, and would appear to be the first thing to fix.

>
> Following is the documentation. This is quite deep in the core, so 
> dont expect a tutorial on this, but you still get some detailled 
> description of the buildbot services lifecycle. (hint: everything is a 
> service in buildbot)
> http://docs.buildbot.net/latest/developer/utils.html#buildbot.util.service.BuildbotService
>
Thanks for the explicit pointer. I was having a bit of trouble 
understanding exactly what checkConfig and reconfigService were supposed 
to do. The config doc in docs/developer doesn't say. And reading the 
code doesn't make it clear. For example, looking at AbstractWorker's 
checkConfig, it might appear that checkConfig is more about 
initialization than checking anything. I'm still not quite sure how I'd 
write checkConfig correctly, though. Any advice?

>
> Every unsolicited code is very welcome by the buildbot community, we 
> are very friendly to all contribution.
>
That would make buildbot nearly unique, in my experience.

>
> Yes, looks simple, but checkConfig/reconfigService is much more 
> flexible. Both functions can be seen like a contructor.
> The first part of the constructor is the part that do the checks, and 
> the second part that actually store the configs inside the object 
> attributes.
> The good think is that the config part can be an inlineCallbacks, so 
> you can call deferred returning function. This is very powerful and 
> you cannot do that with python constructors.
>
> The drawback of this method is that it requires a bit of code 
> duplication, as the two methods must have the exact same footprint.
>
I see that in every case, they do. I'm not sure why that's a 
requirement, though. It would certainly seem to me that a uniform 
interface, such as reconfigServiceWithSibling would be easier to deal 
with than having to explicitly have two functions whose argument lists 
must match. Though I freely admit that my understanding might be colored 
by a rather limited knowledge of python syntax.


> I am not sure if you can't contribute because of company policy or 
> anything, but I would recommend to try and contribute a proper fix.
>
That's unlikely to be a problem. We have contributed to other projects, 
and probably continue to do so.

> I started my work with buildbot by doing such hacks in the prod, and 
> after 4 years we have tons of big hacks in it. We can't upgrade our 
> prod to the new awesomeness of nine, and are stuck with 0.8.7.
> This is unfortunaly the case with some other big companies (watch were 
> my eyes rest.).
>
This doesn't exactly fill me with confidence.

> Buildbot is helping you a lot, so maybe its time to help Buildbot.
>
Frankly, it's not helping me personally. It's a big time sink that keeps 
me from getting my other work done. And if we'd have known that 0.9.0 
was this much less functional than the 0.8.x we were using, I doubt we 
would have upgraded. So far, the only positive feedback I've received on 
our upgrade is that by using multi-master, the UI is much more 
responsive. But no one here likes the new UI (except the 
queue-cancelling button). And many are positively furious at the loss of 
functionality, especially the problem with schedulers not reconfiguring 
properly. Being the messenger, as it were, much of that appears directed 
at me. At least I figured out how to fix our stuck builds. It's a hassle 
to open/close the manhole every time, but at least I can do it.

Neil Gilmore
grammatech.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.buildbot.net/pipermail/users/attachments/20161118/5c696084/attachment.html>