> On Aug. 25, 2015, 6:46 p.m., Joris Van Remoortere wrote:
> > src/tests/maintenance.hpp, lines 78-79
> > <https://reviews.apache.org/r/37325/diff/12/?file=1051209#file1051209line78>
> >
> >     Why do we have to fall down to second percision here?
> >     Can we take an Option<TimeSpec> instead?
> >     Is the start time really optional?

* For testing, I felt it was enough to have second precision.  But it wouldn't 
hurt to have more.
* If we interpret the int64_t as nanoseconds, then we'd get both fields.  At 
the same time, it'll match how we represent time (internally) in the Duration 
class.  The only downside is that we'd be writing a lot more zeros in the tests.
* I think this is a mistake on my part.  In Maintenance.proto 
(https://reviews.apache.org/r/36571/), I set the unavailability field in the 
Maintenance window as optional.

I'll make these changes momentarily.


- Joseph


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


On Aug. 25, 2015, 10:03 a.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37325/
> -----------------------------------------------------------
> 
> (Updated Aug. 25, 2015, 10:03 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, 
> Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-2067 and MESOS-3069
>     https://issues.apache.org/jira/browse/MESOS-2067
>     https://issues.apache.org/jira/browse/MESOS-3069
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Endpoint: /maintenance.schedule
> 
> Registry operation = maintenance::UpdateSchedule
>   Replaces the schedule with the given one.  Also sets all scheduled machines 
> into Draining mode.
> 
> Other changes:
>   Added a note about the "strict" flag.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db 
>   src/master/http.cpp 37d76ee72f6a037f551bf2609e9393e16b496e44 
>   src/master/maintenance.hpp PRE-CREATION 
>   src/master/maintenance.cpp PRE-CREATION 
>   src/master/master.hpp 36c67599ef2c470da8d95f2caf926a154342d2cc 
>   src/master/master.cpp 95207d24db0aa052eb70c4cc7eb75d0611c365cf 
>   src/master/registrar.hpp c6a0655c212646618d93c9c85918af482a9ffd50 
>   src/tests/maintenance.hpp PRE-CREATION 
>   src/tests/master_maintenance_tests.cpp PRE-CREATION 
>   src/tests/registrar_tests.cpp 032e644ee19751b4ce5767d46f474d34ec4b9166 
> 
> Diff: https://reviews.apache.org/r/37325/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> New Tests:
>   RegistrarTest.UpdateMaintenanceSchedule
>     Schedules 3 machines, 1 at a time.  Rearranges schedules.
>     Checks that machines are put into Draining mode.  Removes machines.
>   MasterMaintenanceTest.UpdateSchedule
>     Hits the new endpoint with some valid and invalid schedules.
>     Only tests a subset of invalid schedules (requires other endpoints to 
> fully test).
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Reply via email to