mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Niklas Nielsen" <...@qni.dk>
Subject Re: Review Request 31016: Added slave run task decorator.
Date Wed, 08 Apr 2015 00:59:38 GMT


> On Feb. 22, 2015, 4:48 p.m., Kapil Arya wrote:
> > src/slave/slave.cpp, line 1111
> > <https://reviews.apache.org/r/31016/diff/1/?file=863935#file863935line1111>
> >
> >     Can we instead swap its name with `task_` and move the label decorator higher
(right after entry into this function)?
> >     
> >     This would prevent future developers from accidentally using `task` when they
meant to use `task_`.
> >     
> >     Doing the label decoration before the error checks has an added benefit of generating
notifications for dropped tasks as well for the hook modules.
> 
> Niklas Nielsen wrote:
>     I don't think we should reveal task infos that are failing. How should the module
implementer know the difference between a successfully launched task and a failed one?
>     
>     The pattern is to append '_' as f(), f'(), f''(). So would keep that for consistency.

I inlined the update so I could get rid of the helper (and copy)


> On Feb. 22, 2015, 4:48 p.m., Kapil Arya wrote:
> > src/slave/slave.cpp, lines 3924-3934
> > <https://reviews.apache.org/r/31016/diff/1/?file=863935#file863935line3924>
> >
> >     Should we rather move it close to the runTask definition? Since the two are
logically close.
> 
> Niklas Nielsen wrote:
>     The ordering is slightly inconsistent (should follow the one from the header file).
>     
>     I'd prefer not to cludder the code and methods around runTask() with helpers - there
is a lot of central pieces there already. Are you feeling strongly about moving this up?

Helper should be gone in the latest patch


- Niklas


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


On April 7, 2015, 5:57 p.m., Niklas Nielsen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31016/
> -----------------------------------------------------------
> 
> (Updated April 7, 2015, 5:57 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Kapil Arya.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added decorator which gets invoked on start of runTask() sequence in the slave.
> 
> 
> Diffs
> -----
> 
>   include/mesos/hook.hpp 9ae8b9455a86c7a5cbf4f1d1b1ce88f2811ce35d 
>   src/hook/manager.hpp da813492108974a7e26b366845368517589da876 
>   src/hook/manager.cpp 7a4cb09bc221af502e867cfb7fff2900b599ff1f 
>   src/slave/slave.hpp 19e6b44bc344c0ca509674803f401cbb4e1f47ae 
>   src/slave/slave.cpp 9fec023b643d410f4d511fa6f80e9835bab95b7e 
> 
> Diff: https://reviews.apache.org/r/31016/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Niklas Nielsen
> 
>


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