Re: Review Request 68147: Added agent support to remove local resource providers.

2019-01-17 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68147/#review212123 --- Ship it! Ship It! - Chun-Hung Hsiao On Jan. 17, 2019, 11:17

Re: Review Request 68147: Added agent support to remove local resource providers.

2019-01-17 Thread Benjamin Bannier
> On Jan. 17, 2019, 5:42 a.m., Chun-Hung Hsiao wrote: > > src/slave/http.cpp > > Lines 3355 (patched) > > > > > > Most handlers print this line *before* creating the approvers. Let's > > move this before L3345. >

Re: Review Request 68147: Added agent support to remove local resource providers.

2019-01-17 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68147/ --- (Updated Jan. 17, 2019, 12:17 p.m.) Review request for mesos, Chun-Hung Hsiao,

Re: Review Request 68147: Added agent support to remove local resource providers.

2019-01-16 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68147/#review212103 --- Fix it, then Ship it! src/slave/http.cpp Lines 3355 (patched)

Re: Review Request 68147: Added agent support to remove local resource providers.

2019-01-16 Thread Benjamin Bannier
> On Aug. 18, 2018, 12:29 a.m., Chun-Hung Hsiao wrote: > > src/slave/slave.cpp > > Lines 8195 (patched) > > > > > > Let's validate that there is no task using the resources provided by > > this RP before doing the

Re: Review Request 68147: Added agent support to remove local resource providers.

2019-01-16 Thread Benjamin Bannier
> On Jan. 15, 2019, 6:53 a.m., Chun-Hung Hsiao wrote: > > src/tests/api_tests.cpp > > Lines 7857 (patched) > > > > > > ``` > > Owned detector = master.get()->createDetector(); > > ``` > > And > >

Re: Review Request 68147: Added agent support to remove local resource providers.

2019-01-16 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68147/ --- (Updated Jan. 16, 2019, 11:51 a.m.) Review request for mesos, Chun-Hung Hsiao,

Re: Review Request 68147: Added agent support to remove local resource providers.

2019-01-15 Thread Chun-Hung Hsiao
> On Aug. 17, 2018, 10:29 p.m., Chun-Hung Hsiao wrote: > > src/slave/slave.cpp > > Lines 8195 (patched) > > > > > > Let's validate that there is no task using the resources provided by > > this RP before doing the

Re: Review Request 68147: Added agent support to remove local resource providers.

2019-01-15 Thread Chun-Hung Hsiao
> On Jan. 15, 2019, 5:53 a.m., Chun-Hung Hsiao wrote: > > src/tests/api_tests.cpp > > Lines 7857 (patched) > > > > > > ``` > > Owned detector = master.get()->createDetector(); > > ``` > > And > >

Re: Review Request 68147: Added agent support to remove local resource providers.

2019-01-15 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68147/ --- (Updated Jan. 15, 2019, 4:03 p.m.) Review request for mesos, Chun-Hung Hsiao,

Re: Review Request 68147: Added agent support to remove local resource providers.

2019-01-15 Thread Benjamin Bannier
> On Jan. 15, 2019, 6:42 a.m., Chun-Hung Hsiao wrote: > > src/master/master.cpp > > Lines 11248 (patched) > > > > > > Can you add a comment explaining why it is not necessary to call > >

Re: Review Request 68147: Added agent support to remove local resource providers.

2019-01-15 Thread Benjamin Bannier
> On Jan. 15, 2019, 6:53 a.m., Chun-Hung Hsiao wrote: > > src/tests/api_tests.cpp > > Lines 7857 (patched) > > > > > > ``` > > Owned detector = master.get()->createDetector(); > > ``` > > And > >

Re: Review Request 68147: Added agent support to remove local resource providers.

2019-01-15 Thread Benjamin Bannier
> On Aug. 18, 2018, 12:29 a.m., Chun-Hung Hsiao wrote: > > src/slave/slave.cpp > > Lines 8195 (patched) > > > > > > Let's validate that there is no task using the resources provided by > > this RP before doing the

Re: Review Request 68147: Added agent support to remove local resource providers.

2019-01-15 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68147/ --- (Updated Jan. 15, 2019, 3:53 p.m.) Review request for mesos, Chun-Hung Hsiao,

Re: Review Request 68147: Added agent support to remove local resource providers.

2019-01-14 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68147/#review211997 --- src/tests/api_tests.cpp Lines 7857 (patched)

Re: Review Request 68147: Added agent support to remove local resource providers.

2019-01-14 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68147/#review211993 --- src/master/master.cpp Lines 11248 (patched)

Re: Review Request 68147: Added agent support to remove local resource providers.

2019-01-14 Thread Chun-Hung Hsiao
> On Aug. 17, 2018, 10:29 p.m., Chun-Hung Hsiao wrote: > > src/slave/slave.cpp > > Lines 8195 (patched) > > > > > > Let's validate that there is no task using the resources provided by > > this RP before doing the

Re: Review Request 68147: Added agent support to remove local resource providers.

2018-12-17 Thread Benjamin Bannier
> On Dec. 1, 2018, 4:32 a.m., Greg Mann wrote: > > src/slave/slave.cpp > > Lines 8209-8210 (patched) > > > > > > Some high-level feedback after my first glance at this patch: for the > > MARK_AGENT_GONE call, the

Re: Review Request 68147: Added agent support to remove local resource providers.

2018-11-30 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68147/#review210994 --- src/slave/slave.cpp Lines 8209-8210 (patched)

Re: Review Request 68147: Added agent support to remove local resource providers.

2018-10-31 Thread Benjamin Bannier
> On Aug. 18, 2018, 12:29 a.m., Chun-Hung Hsiao wrote: > > src/slave/slave.cpp > > Lines 7934 (patched) > > > > > > `OPERATION_GONE_BY_OPERATOR` is not a terminal state: > > > >

Re: Review Request 68147: Added agent support to remove local resource providers.

2018-10-25 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68147/ --- (Updated Oct. 25, 2018, 12:47 p.m.) Review request for mesos, Chun-Hung Hsiao,

Re: Review Request 68147: Added agent support to remove local resource providers.

2018-10-17 Thread Benjamin Bannier
> On Aug. 18, 2018, 12:29 a.m., Chun-Hung Hsiao wrote: > > src/slave/slave.cpp > > Lines 8195 (patched) > > > > > > Let's validate that there is no task using the resources provided by > > this RP before doing the

Re: Review Request 68147: Added agent support to remove local resource providers.

2018-08-27 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68147/#review207982 --- Patch looks great! Reviews applied: [68407, 68143, 68144, 68146,

Re: Review Request 68147: Added agent support to remove local resource providers.

2018-08-27 Thread Mesos Reviewbot Windows
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68147/#review207979 --- PASS: Mesos patch 68147 was successfully built and tested.

Re: Review Request 68147: Added agent support to remove local resource providers.

2018-08-20 Thread Chun-Hung Hsiao
> On Aug. 17, 2018, 10:29 p.m., Chun-Hung Hsiao wrote: > > src/slave/slave.cpp > > Lines 7934 (patched) > > > > > > `OPERATION_GONE_BY_OPERATOR` is not a terminal state: > > > >

Re: Review Request 68147: Added agent support to remove local resource providers.

2018-08-20 Thread Benjamin Bannier
> On Aug. 18, 2018, 12:29 a.m., Chun-Hung Hsiao wrote: > > src/slave/slave.cpp > > Lines 7934 (patched) > > > > > > `OPERATION_GONE_BY_OPERATOR` is not a terminal state: > > > >

Re: Review Request 68147: Added agent support to remove local resource providers.

2018-08-20 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68147/ --- (Updated Aug. 20, 2018, 11:52 a.m.) Review request for mesos, Chun-Hung Hsiao

Re: Review Request 68147: Added agent support to remove local resource providers.

2018-08-17 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68147/#review207559 --- src/slave/slave.cpp Lines 7934 (patched)

Re: Review Request 68147: Added agent support to remove local resource providers.

2018-08-17 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68147/ --- (Updated Aug. 17, 2018, 3:54 p.m.) Review request for mesos, Chun-Hung Hsiao

Re: Review Request 68147: Added agent support to remove local resource providers.

2018-08-17 Thread Benjamin Bannier
> On Aug. 15, 2018, 10:54 p.m., Chun-Hung Hsiao wrote: > > src/slave/http.cpp > > Lines 3356 (patched) > > > > > > We should return an `InternalServerError` on failed/discarded. > > Benjamin Bannier wrote: > I

Re: Review Request 68147: Added agent support to remove local resource providers.

2018-08-17 Thread Benjamin Bannier
> On Aug. 17, 2018, 1:27 a.m., Chun-Hung Hsiao wrote: > > src/slave/slave.cpp > > Lines 7934 (patched) > > > > > > This line combined with Line 8079 will crash the agent. Ouch, added

Re: Review Request 68147: Added agent support to remove local resource providers.

2018-08-16 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68147/#review207450 --- src/slave/slave.cpp Lines 7934 (patched)

Re: Review Request 68147: Added agent support to remove local resource providers.

2018-08-16 Thread Chun-Hung Hsiao
> On Aug. 15, 2018, 8:54 p.m., Chun-Hung Hsiao wrote: > > src/slave/http.cpp > > Lines 3356 (patched) > > > > > > We should return an `InternalServerError` on failed/discarded. > > Benjamin Bannier wrote: > I

Re: Review Request 68147: Added agent support to remove local resource providers.

2018-08-16 Thread Benjamin Bannier
> On Aug. 15, 2018, 10:54 p.m., Chun-Hung Hsiao wrote: > > src/slave/http.cpp > > Lines 3356 (patched) > > > > > > We should return an `InternalServerError` on failed/discarded. I don't think this is required as a

Re: Review Request 68147: Added agent support to remove local resource providers.

2018-08-16 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68147/ --- (Updated Aug. 16, 2018, 4:31 p.m.) Review request for mesos, Chun-Hung Hsiao

Re: Review Request 68147: Added agent support to remove local resource providers.

2018-08-15 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68147/#review207351 --- src/slave/http.cpp Lines 3356 (patched)

Re: Review Request 68147: Added agent support to remove local resource providers.

2018-08-15 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68147/ --- (Updated Aug. 15, 2018, 3:53 p.m.) Review request for mesos, Chun-Hung Hsiao

Re: Review Request 68147: Added agent support to remove local resource providers.

2018-08-15 Thread Mesos Reviewbot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68147/#review207312 --- Patch looks great! Reviews applied: [68143, 68144, 68145, 68146,

Re: Review Request 68147: Added agent support to remove local resource providers.

2018-08-14 Thread Chun-Hung Hsiao
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68147/#review207277 --- include/mesos/agent/agent.proto Lines 100 (patched)

Re: Review Request 68147: Added agent support to remove local resource providers.

2018-08-02 Thread Mesos Reviewbot Windows
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68147/#review206789 --- FAIL: Some of the unit tests failed. Please check the relevant

Re: Review Request 68147: Added agent support to remove local resource providers.

2018-08-01 Thread Mesos Reviewbot Windows
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68147/#review206753 --- PASS: Mesos patch 68147 was successfully built and tested.

Review Request 68147: Added agent support to remove local resource providers.

2018-08-01 Thread Benjamin Bannier
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68147/ --- Review request for mesos, Chun-Hung Hsiao and Jan Schlicht. Bugs: MESOS-8403