You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Alexander Rukletsov <ru...@gmail.com> on 2017/02/02 09:58:21 UTC
Review Request 56217: Added support for general checks to default
executor.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56217/
-----------------------------------------------------------
Review request for mesos, Gast�n Kleiman and Vinod Kone.
Bugs: MESOS-6906
https://issues.apache.org/jira/browse/MESOS-6906
Repository: mesos
Description
-------
See summary.
Diffs
-----
src/launcher/default_executor.cpp 97eee05cac8cb1f62d43e2aecc08a8e54e49eac3
Diff: https://reviews.apache.org/r/56217/diff/
Testing
-------
See https://reviews.apache.org/r/56218/
Thanks,
Alexander Rukletsov
Re: Review Request 56217: Added support for general checks to default
executor.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On Feb. 15, 2017, 2:32 p.m., Gast�n Kleiman wrote:
> > src/launcher/default_executor.cpp
> > Lines 931 (patched)
> > <https://reviews.apache.org/r/56217/diff/3/?file=1627713#file1627713line931>
> >
> > I think that it'd be useful to also log the exit status/http status code.
> >
> > Maybe the best way would be to implement the `ostream::operator<<` for `CheckStatusInfo`?
Good idea.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56217/#review165698
-----------------------------------------------------------
On Feb. 8, 2017, 4:57 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56217/
> -----------------------------------------------------------
>
> (Updated Feb. 8, 2017, 4:57 p.m.)
>
>
> Review request for mesos, Gast�n Kleiman and Vinod Kone.
>
>
> Bugs: MESOS-6906
> https://issues.apache.org/jira/browse/MESOS-6906
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/launcher/default_executor.cpp e63cf153831088851863d0956455a024e9bc172a
>
>
> Diff: https://reviews.apache.org/r/56217/diff/3/
>
>
> Testing
> -------
>
> See https://reviews.apache.org/r/56218/
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 56217: Added support for general checks to default
executor.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On Feb. 15, 2017, 2:32 p.m., Gast�n Kleiman wrote:
> > src/launcher/default_executor.cpp
> > Lines 931 (patched)
> > <https://reviews.apache.org/r/56217/diff/3/?file=1627713#file1627713line931>
> >
> > I think that it'd be useful to also log the exit status/http status code.
> >
> > Maybe the best way would be to implement the `ostream::operator<<` for `CheckStatusInfo`?
>
> Alexander Rukletsov wrote:
> Good idea.
Addressed in https://reviews.apache.org/r/57696/
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56217/#review165698
-----------------------------------------------------------
On March 16, 2017, 4:45 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56217/
> -----------------------------------------------------------
>
> (Updated March 16, 2017, 4:45 p.m.)
>
>
> Review request for mesos, Gast�n Kleiman and Vinod Kone.
>
>
> Bugs: MESOS-7249
> https://issues.apache.org/jira/browse/MESOS-7249
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/launcher/default_executor.cpp cbd4f7ecd042e7fa603bd69774d95472df2c896d
>
>
> Diff: https://reviews.apache.org/r/56217/diff/4/
>
>
> Testing
> -------
>
> See https://reviews.apache.org/r/56218/
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 56217: Added support for general checks to default
executor.
Posted by Gastón Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56217/#review165698
-----------------------------------------------------------
src/launcher/default_executor.cpp (line 93)
<https://reviews.apache.org/r/56217/#comment237560>
`s/for a/for the/` to be consistent with the other comments.
src/launcher/default_executor.cpp (line 931)
<https://reviews.apache.org/r/56217/#comment237563>
I think that it'd be useful to also log the exit status/http status code.
Maybe the best way would be to implement the `ostream::operator<<` for `CheckStatusInfo`?
- Gast�n Kleiman
On Feb. 8, 2017, 4:57 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56217/
> -----------------------------------------------------------
>
> (Updated Feb. 8, 2017, 4:57 p.m.)
>
>
> Review request for mesos, Gast�n Kleiman and Vinod Kone.
>
>
> Bugs: MESOS-6906
> https://issues.apache.org/jira/browse/MESOS-6906
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/launcher/default_executor.cpp e63cf153831088851863d0956455a024e9bc172a
>
> Diff: https://reviews.apache.org/r/56217/diff/
>
>
> Testing
> -------
>
> See https://reviews.apache.org/r/56218/
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 56217: Added support for general checks to default
executor.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56217/
-----------------------------------------------------------
(Updated March 23, 2017, 12:10 p.m.)
Review request for mesos, Gast�n Kleiman and Vinod Kone.
Changes
-------
Rebased. NNTR.
Bugs: MESOS-7249
https://issues.apache.org/jira/browse/MESOS-7249
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
src/launcher/default_executor.cpp 6a885af50db6d489194cc9b780fcf1494e853694
Diff: https://reviews.apache.org/r/56217/diff/6/
Changes: https://reviews.apache.org/r/56217/diff/5-6/
Testing
-------
See https://reviews.apache.org/r/56218/
Thanks,
Alexander Rukletsov
Re: Review Request 56217: Added support for general checks to default
executor.
Posted by Gastón Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56217/#review169573
-----------------------------------------------------------
Ship it!
Ship It!
- Gast�n Kleiman
On March 20, 2017, 11:56 a.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56217/
> -----------------------------------------------------------
>
> (Updated March 20, 2017, 11:56 a.m.)
>
>
> Review request for mesos, Gast�n Kleiman and Vinod Kone.
>
>
> Bugs: MESOS-7249
> https://issues.apache.org/jira/browse/MESOS-7249
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/launcher/default_executor.cpp cbd4f7ecd042e7fa603bd69774d95472df2c896d
>
>
> Diff: https://reviews.apache.org/r/56217/diff/5/
>
>
> Testing
> -------
>
> See https://reviews.apache.org/r/56218/
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 56217: Added support for general checks to default
executor.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56217/
-----------------------------------------------------------
(Updated March 20, 2017, 11:56 a.m.)
Review request for mesos, Gast�n Kleiman and Vinod Kone.
Bugs: MESOS-7249
https://issues.apache.org/jira/browse/MESOS-7249
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
src/launcher/default_executor.cpp cbd4f7ecd042e7fa603bd69774d95472df2c896d
Diff: https://reviews.apache.org/r/56217/diff/5/
Changes: https://reviews.apache.org/r/56217/diff/4-5/
Testing
-------
See https://reviews.apache.org/r/56218/
Thanks,
Alexander Rukletsov
Re: Review Request 56217: Added support for general checks to default
executor.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56217/#review169272
-----------------------------------------------------------
Fix it, then Ship it!
src/launcher/default_executor.cpp
Lines 962 (patched)
<https://reviews.apache.org/r/56217/#comment241621>
as discussed in previous review, this is not safe.
src/launcher/default_executor.cpp
Lines 1069 (patched)
<https://reviews.apache.org/r/56217/#comment241622>
LOG(FATAL)
- Vinod Kone
On March 16, 2017, 4:45 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56217/
> -----------------------------------------------------------
>
> (Updated March 16, 2017, 4:45 p.m.)
>
>
> Review request for mesos, Gast�n Kleiman and Vinod Kone.
>
>
> Bugs: MESOS-7249
> https://issues.apache.org/jira/browse/MESOS-7249
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/launcher/default_executor.cpp cbd4f7ecd042e7fa603bd69774d95472df2c896d
>
>
> Diff: https://reviews.apache.org/r/56217/diff/4/
>
>
> Testing
> -------
>
> See https://reviews.apache.org/r/56218/
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 56217: Added support for general checks to default
executor.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56217/
-----------------------------------------------------------
(Updated March 16, 2017, 4:45 p.m.)
Review request for mesos, Gast�n Kleiman and Vinod Kone.
Bugs: MESOS-7249
https://issues.apache.org/jira/browse/MESOS-7249
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
src/launcher/default_executor.cpp cbd4f7ecd042e7fa603bd69774d95472df2c896d
Diff: https://reviews.apache.org/r/56217/diff/4/
Changes: https://reviews.apache.org/r/56217/diff/3-4/
Testing
-------
See https://reviews.apache.org/r/56218/
Thanks,
Alexander Rukletsov
Re: Review Request 56217: Added support for general checks to default
executor.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56217/
-----------------------------------------------------------
(Updated Feb. 8, 2017, 4:57 p.m.)
Review request for mesos, Gast�n Kleiman and Vinod Kone.
Bugs: MESOS-6906
https://issues.apache.org/jira/browse/MESOS-6906
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
src/launcher/default_executor.cpp e63cf153831088851863d0956455a024e9bc172a
Diff: https://reviews.apache.org/r/56217/diff/
Testing
-------
See https://reviews.apache.org/r/56218/
Thanks,
Alexander Rukletsov
Re: Review Request 56217: Added support for general checks to default
executor.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56217/
-----------------------------------------------------------
(Updated Feb. 8, 2017, 3:22 p.m.)
Review request for mesos, Gast�n Kleiman and Vinod Kone.
Bugs: MESOS-6906
https://issues.apache.org/jira/browse/MESOS-6906
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
src/launcher/default_executor.cpp e63cf153831088851863d0956455a024e9bc172a
Diff: https://reviews.apache.org/r/56217/diff/
Testing
-------
See https://reviews.apache.org/r/56218/
Thanks,
Alexander Rukletsov