Code review comment for lp://qastaging/~fginther/jenkins-launchpad-plugin/wait-for-node

Revision history for this message
Martin Mrazik (mrazik) wrote :

Few things:

1. Can we make the timeout arg optional with some reasonable default value? It will make the use of this easier as I won't need to think of what "reasonable timeout" is. Not quite sure what the default should be but I would expect something like 2 hours.

2. I would prefer an "--mark-offline" option (maybe on by default) which would put the node to maintenance so no new jobs will be scheduled. You will need to do this anyway and if we do it directly in this script we will shorten the window where a race condition can occur. After the node is in maintenance I would do the check for idle again just to make sure nothing was scheduled while we were doing the lock. Once --mark-offline is there it would be nice to have --mark-online which would put the node back online again without any additional checks. That way all I need is this python script and the rest can be done in shell as a separate build step.

3. I wonder if "def wait_for_node_idle" shouldn't be in jenkinsutils.py (as well as any helpers for making the node online/offline).

review: Needs Fixing

« Back to merge proposal