mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alexander Rukletsov" <ruklet...@gmail.com>
Subject Re: Review Request 38161: Replaced a hard-coded number for registration backoff with a proper constant.
Date Fri, 02 Oct 2015 13:01:56 GMT


> On Oct. 1, 2015, 6:08 p.m., Ben Mahler wrote:
> > src/tests/fault_tolerance_tests.cpp, lines 650-651
> > <https://reviews.apache.org/r/38161/diff/4/?file=1088390#file1088390line650>
> >
> >     I don't think we need this second sentence, since the DEFAULT expresses this
pretty clearly, no?

My personal preference is to be more verbose. I think for a project newbie it's hard to grasp
that `MesosSchedulerDriver driver2(&sched2, framework2, master.get(), DEFAULT_CREDENTIAL);`
sets a registration backoff to a certain value which we use later on to advance the clock.
However, I also see your point => removing the sentence.


> On Oct. 1, 2015, 6:08 p.m., Ben Mahler wrote:
> > src/tests/fault_tolerance_tests.cpp, lines 1547-1551
> > <https://reviews.apache.org/r/38161/diff/4/?file=1088390#file1088390line1547>
> >
> >     It looks inconsistent to do this here but not in the other tests this patch
touches. Now that the variable name includes DEFAULT, how about we remove the explicit setting
here?

We do not use this value for `Clock::advance()` in other tests. Though it is indeed inconsistent,
explicit setting ensures we advance for the same duration as used by the agent. If we remove
it, `CreateSlaveFlags()` may change this value.


- Alexander


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


On Oct. 1, 2015, 10:07 a.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38161/
> -----------------------------------------------------------
> 
> (Updated Oct. 1, 2015, 10:07 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Prepend a constant regulating the backoff factor with DEFAULT_ prefix for clarity, use
it in tests where appropriate and extend comments for posterity.
> 
> 
> Diffs
> -----
> 
>   src/sched/constants.hpp ac497b2abc9fbeab713b312cf37963d7de28ed50 
>   src/sched/constants.cpp 517ca5cb0dad49d775d65c9ffabc849ff85347a0 
>   src/sched/flags.hpp 4e0d56f61b6877254161112b288ae4a9c579e846 
>   src/slave/constants.hpp df18676f17f2277e3c38432b76f16c5f9cb08341 
>   src/slave/constants.cpp cf3ee7bbc252364a1b73731feab6a9da68ee1f55 
>   src/slave/flags.cpp 029aa1eb00e1aa3e92c2155925022c17fd905862 
>   src/tests/fault_tolerance_tests.cpp c97bc4691f9bac4a8677e6d2247be96ee9674b57 
> 
> Diff: https://reviews.apache.org/r/38161/diff/
> 
> 
> Testing
> -------
> 
> make check (Mac OS 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message