Re: Review Request 71978: Updated all GET_* v1 master calls to be served in parallel.

2020-01-14 Thread Andrei Sekretenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71978/#review219252 --- Fix it, then Ship it! Can you also adjust the commit message to

Re: Review Request 71991: Added the 'TASK_RESOURCE_LIMITS' agent capability.

2020-01-14 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71991/#review219255 --- Patch looks great! Reviews applied: [71991] Passed command: expo

Review Request 71996: Fixed issues flagged by cpplint.

2020-01-14 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71996/ --- Review request for mesos, Andrei Sekretenko and Benjamin Mahler. Repository: me

Re: Review Request 71996: Fixed issues flagged by cpplint.

2020-01-14 Thread Andrei Sekretenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71996/#review219256 --- Ship it! Ship It! - Andrei Sekretenko On Jan. 14, 2020, 4:42

Re: Review Request 71978: Updated all GET_* v1 master calls to be served in parallel.

2020-01-14 Thread Benjamin Mahler
> On Jan. 14, 2020, 12:52 p.m., Andrei Sekretenko wrote: > > src/master/master.hpp > > Lines 1388-1417 (patched) > > > > > > Consider making all those `jsonify/serialize` methods (except for > > `*getState` private

Re: Review Request 71858: WIP: Set resource limits when launching executor container.

2020-01-14 Thread Greg Mann
> On Jan. 7, 2020, 9:43 p.m., Greg Mann wrote: > > src/slave/slave.cpp > > Lines 3140-3201 (patched) > > > > > > I think maybe this logic could be easier to read if we do something > > like: > > > > ``` >

Re: Review Request 71858: WIP: Set resource limits when launching executor container.

2020-01-14 Thread Greg Mann
> On Jan. 7, 2020, 9:43 p.m., Greg Mann wrote: > > src/slave/slave.cpp > > Lines 3623 (patched) > > > > > > Should this be an `Option`? So that we can only set > > `containerConfig.limits` when limits have actually

Re: Review Request 71858: WIP: Set resource limits when launching executor container.

2020-01-14 Thread Qian Zhang
> On Jan. 8, 2020, 5:43 a.m., Greg Mann wrote: > > src/slave/slave.cpp > > Lines 3140-3201 (patched) > > > > > > I think maybe this logic could be easier to read if we do something > > like: > > > > ``` >

Re: Review Request 71858: WIP: Set resource limits when launching executor container.

2020-01-14 Thread Greg Mann
> On Jan. 7, 2020, 9:43 p.m., Greg Mann wrote: > > src/slave/slave.cpp > > Lines 3140-3201 (patched) > > > > > > I think maybe this logic could be easier to read if we do something > > like: > > > > ``` >

Re: Review Request 71858: WIP: Set resource limits when launching executor container.

2020-01-14 Thread Greg Mann
> On Jan. 7, 2020, 9:43 p.m., Greg Mann wrote: > > src/slave/slave.cpp > > Lines 3623 (patched) > > > > > > Should this be an `Option`? So that we can only set > > `containerConfig.limits` when limits have actually