[devel at bb.net] [users at bb.net] Another anecdote from 0.9.2 scheduler modification.

[Pierre Tardy]

Hi Neil,

Such discussion are imho easiest to be in the PR or emails rather in trac
(we will be getting rid of trac soon anyway), but this is more suitable in
devel@
[removing users at bb.net and switch to dev at bb.net]


Indeed ForceScheduler is weird and is very different from the other
Schedulers even if it derives from BaseScheduler.
The properties needs to be a list in order to have a defined order, so that
the UI can be specified exactly.
This was designed at a time were OrderedDict was not available in all the
python we supported, and now it is legacy. :-(
The properties from ForceScheduler are not really properties, they need to
be BaseParameters subclasses

I am not sure exactly how we should fix this. Would need to look at the
code (hint: submit your current version :) )

- Do no call super.checkConfig in ForceScheduler ?
- Change forcescheduler to use OrderedDict instead, and make it translate
the properties list to an OrderedDict if it is a list (to support legacy) ?

I think later is the way to go eventually, but we might want to split this
task for later as I dont think this is a good idea to make your PR too big.

Pierre

Le lun. 2 janv. 2017 à 21:45, Neil Gilmore <ngilmore at grammatech.com> a
écrit :

> Hi all,
>
> As always, thanks for the advice.
>
> I think I've found an irregularity in ForceScheduler (naturally, this
> would be netter in trac...). If nothing else, it renders some of the
> documentation on Schedulers to be incorrect (I really can't say which
> way this should fall). I've run into it while adding back
> reconfiguration to the Force Scheduler.
>
> It concerns the 'properties' argument. On the one hand, BaseScheduler
> expects this to be a dictionary, and will in fact force it to an empty
> dictionary if it isn't passed in. One the other, ForceScheduler expects
> it to be a list, and will force it to a list if it isn't passed in.
> Additionally, at least in 0.9.2, ForceScheduler.__init__() does not pass
> its properties argument to BaseScheduler.__init__(). Now, it certainly
> could be that ForceScheduler really doesn't implement the common
> 'properties', preferring its own instead.
>
> But this leads to problems when we attempt to eliminate the __init__()
> functions in favor of CheckConfig() and reconfigService(). We should
> preserve the ForceScheduler.__init__() properties (the list kind) in
> order to check them in checkConfig() and assign them in
> reconfigService(). But BaseScheduler already has 'properties' (the
> dictionary kind).
>
> I think for now, I'll be introducing something like forceProperties in
> __init__() which will end up passed in kwargs. This won't affect the
> current interface to ForceScheduler, and will preserve the original
> properties (the list kind) to be used later. checkConfig and
> reconfigService will use it.
>
> I'll also point out that ForceScheduler's check for name being a unicode
> string is incorrect. If passed a unicode string, it fails. The type
> checked against is 'str', which is not 'unicode'.
>
> So far, I can start a buildbot with any of the variety of schedulers. I
> do need to test reconfiguration to make sure everything actually
> reconfigures.
>
> Then I'll need to submit it and hope the devs like it.
>
> Neil Gilmore
> grammatech.com
> _______________________________________________
> users mailing list
> users at buildbot.net
> https://lists.buildbot.net/mailman/listinfo/users
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.buildbot.net/pipermail/devel/attachments/20170103/125c9f9c/attachment.html>