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

Pierre Tardy tardyp at gmail.com
Wed Jan 4 12:41:40 UTC 2017


Le mar. 3 janv. 2017 à 16:57, Neil Gilmore <ngilmore at grammatech.com> a
écrit :

Hi Pierre,

Thanks for the reply.

The current status of my code is that all the schedulers now support
checkConfig and reconfigService. I can also (after some wrangling) start
buildbot using any of the schedulers. I have yet to test reconfiguration,
and I have not run any of the current buildbot unit tests. Nor have I done
any of the github stuff. I did subscribe to the dev list (it was really
just a matter of time).


I highly suggest to start with the unit tests, as once it is understood
help you to be much more productive than typing buildbot reconfigure all
the time.
 Also, obviously creating an account of github and pushing your code will
help me to help you.


My current code (which appears to work) adds another argument for
ForceSchedulers 'properties'. The __init__() function uses 'properties',
but then calls BaseScheduler.__init__() using the new argument, which ends
up in kwargs for most of the trip around to ForceScheduler.checkConfig().
The code cribbed from the old __init__() is adjusted to use the new name.
Thus, the current API and behavior are preserved.

We have to make sure we keep the same API, but removing properties from
super's kwargs might indeed do the trick.


I don't think not calling super.checkConfig is a good idea. We still need
to check the rest of the config against the base classes (in this case
BaseScheduler and BuildbotService). Incidentally, my current code only uses
super if the code in __init__() used it. Most of the classes used explicit
bases, so I left that. It's a decent candidate for a later change.

The current code changes are pretty extensive, yes, because they affect
every scheduler class. I think it might be good to stage the PRs this way:

1. Regularize compare_attrs for schedulers. In 0.9.2, several schedulers
appear to have a problem where they do not include their base classes'
compare_attrs. This wasn't as important when there was no effective
reconfiguration.

There is indeed a need for cleanup, but not the way you might think of it.
compare_attr uses reflect.accumulateClassList, which does take in account
all compare_attr of all the subclasses.
https://github.com/buildbot/buildbot/blob/master/master/buildbot/util/__init__.py#L136
Some classes are explicitly adding their parents compare_attr but this is
either a mistake or a legacy (I am not sure, this precedes my involvment in
this project)


2. Add the checkConfig and reconfigService functions, but leave them empty.
This will at least ensure that I have those setup up properly.

+ remove the __init__. We should always use the BuildbotService class
__init__ for correct use of checkConfig/reconfigService


3. Fill the functions.

My current task is to exercise actual reconfigurability. This will involve
learning how to write a buildbot test, which will probably be predicated on
learning how to run the current tests.


Some useful links for that:
http://docs.buildbot.net/latest/developer/tests.html
The basic step to run the tests is the command:
trial buildbot

you might also like https://pypi.python.org/pypi/cctrial

pip install cctrial
cctrial -sf buildbot

cctrial -sf allows a continuous development workflow which run only the
tests that are modified by the files you just worked on, and the test
automatically re-run after file save until it passes.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.buildbot.net/pipermail/devel/attachments/20170104/f38f04b0/attachment.html>


More information about the devel mailing list