Re: Review Request 45327: Drop HTTP Status `404` and `307` in the scheduler library.

2016-03-25 Thread Vinod Kone 

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


Ship it!




Ship It!

- Vinod Kone


On March 25, 2016, 2:28 a.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45327/
> ---
> 
> (Updated March 25, 2016, 2:28 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5020
> https://issues.apache.org/jira/browse/MESOS-5020
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix drops `404 Not Found` and `307 Temporary Redirect` in
> the scheduler library, instead of treating them as Event::ERROR.
> (MESOS-5020)
> 
> 
> Diffs
> -
> 
>   src/scheduler/scheduler.cpp 1f52a423c8c83c58d38ba6731ab2625136a271bb 
> 
> Diff: https://reviews.apache.org/r/45327/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45327: Drop HTTP Status `404` and `307` in the scheduler library.

2016-03-25 Thread Mesos ReviewBot 

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



Patch looks great!

Reviews applied: [45327]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On March 25, 2016, 2:28 a.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45327/
> ---
> 
> (Updated March 25, 2016, 2:28 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5020
> https://issues.apache.org/jira/browse/MESOS-5020
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix drops `404 Not Found` and `307 Temporary Redirect` in
> the scheduler library, instead of treating them as Event::ERROR.
> (MESOS-5020)
> 
> 
> Diffs
> -
> 
>   src/scheduler/scheduler.cpp 1f52a423c8c83c58d38ba6731ab2625136a271bb 
> 
> Diff: https://reviews.apache.org/r/45327/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45327: Drop HTTP Status `404` and `307` in the scheduler library.

2016-03-24 Thread Yong Tang 


> On March 25, 2016, 1:53 a.m., Anand Mazumdar wrote:
> > LGTM, just some minor code comments.

Thanks a lot for the review Anand. I just updated the review request with the 
fixes. Let me know if there are other issues.


- Yong


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


On March 25, 2016, 2:28 a.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45327/
> ---
> 
> (Updated March 25, 2016, 2:28 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5020
> https://issues.apache.org/jira/browse/MESOS-5020
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix drops `404 Not Found` and `307 Temporary Redirect` in
> the scheduler library, instead of treating them as Event::ERROR.
> (MESOS-5020)
> 
> 
> Diffs
> -
> 
>   src/scheduler/scheduler.cpp 1f52a423c8c83c58d38ba6731ab2625136a271bb 
> 
> Diff: https://reviews.apache.org/r/45327/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45327: Drop HTTP Status `404` and `307` in the scheduler library.

2016-03-24 Thread Yong Tang 

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

(Updated March 25, 2016, 2:28 a.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

Address comment issues based on the feedback from Anand.


Bugs: MESOS-5020
https://issues.apache.org/jira/browse/MESOS-5020


Repository: mesos


Description
---

This fix drops `404 Not Found` and `307 Temporary Redirect` in
the scheduler library, instead of treating them as Event::ERROR.
(MESOS-5020)


Diffs (updated)
-

  src/scheduler/scheduler.cpp 1f52a423c8c83c58d38ba6731ab2625136a271bb 

Diff: https://reviews.apache.org/r/45327/diff/


Testing
---

make check (Ubuntu 14.04)


Thanks,

Yong Tang

Re: Review Request 45327: Drop HTTP Status `404` and `307` in the scheduler library.

2016-03-24 Thread Anand Mazumdar 

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


Fix it, then Ship it!




LGTM, just some minor code comments.


src/scheduler/scheduler.cpp (line 547)


Let's keep the wording consistent with the executor library.

```cpp
// This could happen if the master libprocess process has not yet set up 
HTTP routes.
```

Also was missing a period at the end.



src/scheduler/scheduler.cpp (line 554)


```cpp
// This could happen if the detector detects a new leading master before 
master itself realizes it (e.g., ZK watch delay).
```


- Anand Mazumdar


On March 24, 2016, 11:21 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45327/
> ---
> 
> (Updated March 24, 2016, 11:21 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5020
> https://issues.apache.org/jira/browse/MESOS-5020
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix drops `404 Not Found` and `307 Temporary Redirect` in
> the scheduler library, instead of treating them as Event::ERROR.
> (MESOS-5020)
> 
> 
> Diffs
> -
> 
>   src/scheduler/scheduler.cpp 1f52a423c8c83c58d38ba6731ab2625136a271bb 
> 
> Diff: https://reviews.apache.org/r/45327/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Review Request 45327: Drop HTTP Status `404` and `307` in the scheduler library.

2016-03-24 Thread Yong Tang 

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

Review request for mesos, Anand Mazumdar and Vinod Kone.


Bugs: MESOS-5020
https://issues.apache.org/jira/browse/MESOS-5020


Repository: mesos


Description
---

This fix drops `404 Not Found` and `307 Temporary Redirect` in
the scheduler library, instead of treating them as Event::ERROR.
(MESOS-5020)


Diffs
-

  src/scheduler/scheduler.cpp 1f52a423c8c83c58d38ba6731ab2625136a271bb 

Diff: https://reviews.apache.org/r/45327/diff/


Testing
---

make check (Ubuntu 14.04)


Thanks,

Yong Tang