> On May 21, 2014, 9:41 a.m., Tobias Weingartner wrote:
> > The API seems a bit strange to me here.  In particular, there is a 
> > HostMaintenance class, and methods:
> > 
> > HostMaintenance.start_maintenance(host_list)
> > HostMaintenance.end_maintenance(host_list)
> > ...
> > 
> > Which all seems very repetitive to me. It might make more sense to have 
> > HostMaintenance's constructor take a list of hosts (already grouped) to 
> > operate on:
> > 
> > maint = HostMaintenance(possibly_grouped_host_list)
> > maint.status(host_subset=None) => return status of each host in 
> > possibly_grouped_host_list, or a subset thereof if given, raise hell if 
> > subset not in possibly_grouped_host_list
> > maint.start(host_subset=None) => return some sort of status of the 
> > operation, or nothing and just raise hell?
> > maint.drain(host_subset=None) => return next drained batch/host, do in a 
> > loop, like using a generator
> > maint.end(host_subset=None) => Usually given the output of maint.drain()
> > 
> > A usecase would then look something like:
> > 
> > maint = HostMaintenance([
> >   ['host1', 'host2', 'host3'],  # Batch 1
> >   ['host4', 'host5'],           # Batch 2 (followed by single hosts)
> >   'host6', 'host7', 'host8', 'host9'])
> > print "Current status:", maint.status()
> > maint.start()
> > for batch in maint.drain():
> >   for host in batch:
> >     # Do the funky chicken
> >   maint.end(batch)
> > 
> > Possibly having a HostMaintenanceController class to batch/coordinate/etc, 
> > using the HostMaintenance class to do this low level work, taking things 
> > like a grouping function, inter-batch delays, inter-host delays, "funky 
> > chicken" function to call for each host/batch, ability to monitor progress, 
> > possibly a "big red button, PANIC stop" ability.
> > 
> > Shipping this for now, but a refactor may be in order.

captured via https://issues.apache.org/jira/browse/AURORA-501


> On May 21, 2014, 9:41 a.m., Tobias Weingartner wrote:
> > src/main/python/apache/aurora/admin/host_maintenance.py, line 43
> > <https://reviews.apache.org/r/20285/diff/2/?file=585968#file585968line43>
> >
> >     underneath it.

Done.


> On May 21, 2014, 9:41 a.m., Tobias Weingartner wrote:
> > src/main/python/apache/aurora/admin/host_maintenance.py, line 53
> > <https://reviews.apache.org/r/20285/diff/2/?file=585968#file585968line53>
> >
> >     I think this function really belongs in somewhere else, possibly in a 
> > "Host" class of some sort.  Or even a "HostSet" class.

This has since been moved out to 'base' as other components are using it.


> On May 21, 2014, 9:41 a.m., Tobias Weingartner wrote:
> > src/main/python/apache/aurora/admin/host_maintenance.py, line 65
> > <https://reviews.apache.org/r/20285/diff/2/?file=585968#file585968line65>
> >
> >     As a dictionary is not order preserving, would it make sense to use a 
> > list of lists?

In practice I haven't found the need for this (yet?) but am open to it changing 
as needed going forward.


> On May 21, 2014, 9:41 a.m., Tobias Weingartner wrote:
> > src/main/python/apache/aurora/admin/host_maintenance.py, line 115
> > <https://reviews.apache.org/r/20285/diff/2/?file=585968#file585968line115>
> >
> >     s/give/given

Good catch!


- Joe


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20285/#review43628
-----------------------------------------------------------


On May 20, 2014, 8:51 p.m., Joe Smith wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20285/
> -----------------------------------------------------------
> 
> (Updated May 20, 2014, 8:51 p.m.)
> 
> 
> Review request for Aurora, David Robinson, Kevin Sweeney, Tobias Weingartner, 
> and Bill Farner.
> 
> 
> Bugs: AURORA-318
>     https://issues.apache.org/jira/browse/AURORA-318
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Add way more docstrings as  well as unit tests for the HostMaintenance api.
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/admin/host_maintenance.py 
> 16c18bc563c61e37055a8ad81082d60d1662afb2 
>   src/test/python/apache/aurora/admin/BUILD 
> 17aac54d636024a8e131eb2e4d996c2e15401de1 
>   src/test/python/apache/aurora/admin/test_host_maintenance.py 
> 18bf6223adb6b64cc76c2810abc317ef063e16e2 
> 
> Diff: https://reviews.apache.org/r/20285/diff/
> 
> 
> Testing
> -------
> 
> 16:39:58 incubator-aurora $ ./pants ./src/test/python/apache/aurora/admin:all
> Build operating on targets: 
> OrderedSet([PythonTestSuite(src/test/python/apache/aurora/admin/BUILD:all)])
> ============================================================================================================
>  test session starts 
> =============================================================================================================
> platform darwin -- Python 2.6.8 -- py-1.4.20 -- pytest-2.5.2
> collected 9 items 
> 
> src/test/python/apache/aurora/admin/test_host_maintenance.py .........
> 
> ==========================================================================================================
>  9 passed in 0.67 seconds 
> ==========================================================================================================
> src.test.python.apache.aurora.admin.host_maintenance                          
>   .....   SUCCESS
> 
> 
> Thanks,
> 
> Joe Smith
> 
>

Reply via email to