Re: Review Request 36431: Show stdout/stderr when using mesos-execute.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36431/#review141488 --- Closing this review due to inactivity. Please see our [guidelines](https://github.com/apache/mesos/blob/master/docs/reopening-reviews.md) for reopening reviews. - Joris Van Remoortere On Oct. 3, 2015, 4:12 p.m., haosdent huang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36431/ > --- > > (Updated Oct. 3, 2015, 4:12 p.m.) > > > Review request for mesos, Adam B and Michael Park. > > > Bugs: MESOS-1084 > https://issues.apache.org/jira/browse/MESOS-1084 > > > Repository: mesos > > > Description > --- > > Show stdout/stderr when using mesos-execute. > > > Diffs > - > > src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc > > Diff: https://reviews.apache.org/r/36431/diff/ > > > Testing > --- > > make check > >  > > > Thanks, > > haosdent huang > >
Re: Review Request 36431: Show stdout/stderr when using mesos-execute.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36431/#review101417 --- Patch looks great! Reviews applied: [36431] All tests passed. - Mesos ReviewBot On Oct. 3, 2015, 4:12 p.m., haosdent huang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36431/ > --- > > (Updated Oct. 3, 2015, 4:12 p.m.) > > > Review request for mesos, Adam B and Michael Park. > > > Bugs: MESOS-1084 > https://issues.apache.org/jira/browse/MESOS-1084 > > > Repository: mesos > > > Description > --- > > Show stdout/stderr when using mesos-execute. > > > Diffs > - > > src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc > > Diff: https://reviews.apache.org/r/36431/diff/ > > > Testing > --- > > make check > >  > > > Thanks, > > haosdent huang > >
Re: Review Request 36431: Show stdout/stderr when using mesos-execute.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36431/ --- (Updated Oct. 3, 2015, 4:12 p.m.) Review request for mesos, Adam B and Michael Park. Changes --- Rebase Bugs: MESOS-1084 https://issues.apache.org/jira/browse/MESOS-1084 Repository: mesos Description --- Show stdout/stderr when using mesos-execute. Diffs (updated) - src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc Diff: https://reviews.apache.org/r/36431/diff/ Testing --- make check  Thanks, haosdent huang
Re: Review Request 36431: Show stdout/stderr when using mesos-execute.
> On Aug. 6, 2015, 11:43 p.m., Marco Massenzio wrote: > > Before we go too far down this path, can you please see > > https://reviews.apache.org/r/36424/ and see whether it does what you are > > trying to achieve here? > > (maybe not, I'm not familiar at all with `cli` - just asking) > > haosdent huang wrote: > Hi, @marco .Thank you for your remind. I take a look of > [r36424](https://reviews.apache.org/r/36424/) . This review is to execute > command in local and get its stdout/stderr, right? If so, it is different > with my patch. My patch is try to get stdout/stderr of task from slave. It > fetch stdout/stderr from slave, maybe in remote machine. > > Marco Massenzio wrote: > Got it. > Sounds good. > > Will do a review when I have some time (next week it's going to be tough > with 0.24 coming up) - at a quick glance, it looks like a pretty long method > (also, inside a .hpp file? not sure, I just skimmed it): if you don't want to > wait for me, you can start thinking about how to break it up in smaller, > helper methods; remove all the hard-coded "magic numbers" for timeouts > (should be either constants or, even better, user configurable, with sensible > defaults); and minimize the use of `state.json` if you can (or even use the > `summary` endpoint) as it's a "heavy" endpoint and can be heavy on a master > with many slaves/frameworks/tasks. Thank you. Let me update it. - haosdent --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36431/#review94460 --- On Aug. 7, 2015, 10:37 a.m., haosdent huang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36431/ > --- > > (Updated Aug. 7, 2015, 10:37 a.m.) > > > Review request for mesos, Adam B and Michael Park. > > > Bugs: MESOS-1084 > https://issues.apache.org/jira/browse/MESOS-1084 > > > Repository: mesos > > > Description > --- > > Show stdout/stderr when using mesos-execute. > > > Diffs > - > > src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc > > Diff: https://reviews.apache.org/r/36431/diff/ > > > Testing > --- > > make check > >  > > > Thanks, > > haosdent huang > >
Re: Review Request 36431: Show stdout/stderr when using mesos-execute.
> On Aug. 6, 2015, 11:43 p.m., Marco Massenzio wrote: > > Before we go too far down this path, can you please see > > https://reviews.apache.org/r/36424/ and see whether it does what you are > > trying to achieve here? > > (maybe not, I'm not familiar at all with `cli` - just asking) > > haosdent huang wrote: > Hi, @marco .Thank you for your remind. I take a look of > [r36424](https://reviews.apache.org/r/36424/) . This review is to execute > command in local and get its stdout/stderr, right? If so, it is different > with my patch. My patch is try to get stdout/stderr of task from slave. It > fetch stdout/stderr from slave, maybe in remote machine. Got it. Sounds good. Will do a review when I have some time (next week it's going to be tough with 0.24 coming up) - at a quick glance, it looks like a pretty long method (also, inside a .hpp file? not sure, I just skimmed it): if you don't want to wait for me, you can start thinking about how to break it up in smaller, helper methods; remove all the hard-coded "magic numbers" for timeouts (should be either constants or, even better, user configurable, with sensible defaults); and minimize the use of `state.json` if you can (or even use the `summary` endpoint) as it's a "heavy" endpoint and can be heavy on a master with many slaves/frameworks/tasks. - Marco --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36431/#review94460 --- On Aug. 7, 2015, 10:37 a.m., haosdent huang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36431/ > --- > > (Updated Aug. 7, 2015, 10:37 a.m.) > > > Review request for mesos, Adam B and Michael Park. > > > Bugs: MESOS-1084 > https://issues.apache.org/jira/browse/MESOS-1084 > > > Repository: mesos > > > Description > --- > > Show stdout/stderr when using mesos-execute. > > > Diffs > - > > src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc > > Diff: https://reviews.apache.org/r/36431/diff/ > > > Testing > --- > > make check > >  > > > Thanks, > > haosdent huang > >
Re: Review Request 36431: Show stdout/stderr when using mesos-execute.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36431/#review94530 --- Patch looks great! Reviews applied: [36431] All tests passed. - Mesos ReviewBot On Aug. 7, 2015, 10:37 a.m., haosdent huang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36431/ > --- > > (Updated Aug. 7, 2015, 10:37 a.m.) > > > Review request for mesos, Adam B and Michael Park. > > > Bugs: MESOS-1084 > https://issues.apache.org/jira/browse/MESOS-1084 > > > Repository: mesos > > > Description > --- > > Show stdout/stderr when using mesos-execute. > > > Diffs > - > > src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc > > Diff: https://reviews.apache.org/r/36431/diff/ > > > Testing > --- > > make check > >  > > > Thanks, > > haosdent huang > >
Re: Review Request 36431: Show stdout/stderr when using mesos-execute.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36431/ --- (Updated Aug. 7, 2015, 10:37 a.m.) Review request for mesos, Adam B and Michael Park. Bugs: MESOS-1084 https://issues.apache.org/jira/browse/MESOS-1084 Repository: mesos Description --- Show stdout/stderr when using mesos-execute. Diffs - src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc Diff: https://reviews.apache.org/r/36431/diff/ Testing (updated) --- make check  Thanks, haosdent huang
Re: Review Request 36431: Show stdout/stderr when using mesos-execute.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36431/ --- (Updated Aug. 7, 2015, 10:30 a.m.) Review request for mesos, Adam B and Michael Park. Changes --- rebase from current master. Bugs: MESOS-1084 https://issues.apache.org/jira/browse/MESOS-1084 Repository: mesos Description --- Show stdout/stderr when using mesos-execute. Diffs (updated) - src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc Diff: https://reviews.apache.org/r/36431/diff/ Testing --- make check Thanks, haosdent huang
Re: Review Request 36431: Show stdout/stderr when using mesos-execute.
> On Aug. 6, 2015, 11:43 p.m., Marco Massenzio wrote: > > Before we go too far down this path, can you please see > > https://reviews.apache.org/r/36424/ and see whether it does what you are > > trying to achieve here? > > (maybe not, I'm not familiar at all with `cli` - just asking) Hi, @marco .Thank you for your remind. I take a look of [r36424](https://reviews.apache.org/r/36424/) . This review is to execute command in local and get its stdout/stderr, right? If so, it is different with my patch. My patch is try to get stdout/stderr of task from slave. It fetch stdout/stderr from slave, maybe in remote machine. - haosdent --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36431/#review94460 --- On Aug. 4, 2015, 9:33 a.m., haosdent huang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36431/ > --- > > (Updated Aug. 4, 2015, 9:33 a.m.) > > > Review request for mesos, Adam B and Michael Park. > > > Bugs: MESOS-1084 > https://issues.apache.org/jira/browse/MESOS-1084 > > > Repository: mesos > > > Description > --- > > Show stdout/stderr when using mesos-execute. > > > Diffs > - > > src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc > > Diff: https://reviews.apache.org/r/36431/diff/ > > > Testing > --- > > make check > > > Thanks, > > haosdent huang > >
Re: Review Request 36431: Show stdout/stderr when using mesos-execute.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36431/#review94459 --- Before we go too far down this path, can you please see https://reviews.apache.org/r/36424/ and see whether it does what you are trying to achieve here? (maybe not, I'm not familiar at all with `cli` - just asking) - Marco Massenzio On Aug. 4, 2015, 9:33 a.m., haosdent huang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36431/ > --- > > (Updated Aug. 4, 2015, 9:33 a.m.) > > > Review request for mesos, Adam B and Michael Park. > > > Bugs: MESOS-1084 > https://issues.apache.org/jira/browse/MESOS-1084 > > > Repository: mesos > > > Description > --- > > Show stdout/stderr when using mesos-execute. > > > Diffs > - > > src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc > > Diff: https://reviews.apache.org/r/36431/diff/ > > > Testing > --- > > make check > > > Thanks, > > haosdent huang > >
Re: Review Request 36431: Show stdout/stderr when using mesos-execute.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36431/#review94461 --- - Marco Massenzio On Aug. 4, 2015, 9:33 a.m., haosdent huang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36431/ > --- > > (Updated Aug. 4, 2015, 9:33 a.m.) > > > Review request for mesos, Adam B and Michael Park. > > > Bugs: MESOS-1084 > https://issues.apache.org/jira/browse/MESOS-1084 > > > Repository: mesos > > > Description > --- > > Show stdout/stderr when using mesos-execute. > > > Diffs > - > > src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc > > Diff: https://reviews.apache.org/r/36431/diff/ > > > Testing > --- > > make check > > > Thanks, > > haosdent huang > >
Re: Review Request 36431: Show stdout/stderr when using mesos-execute.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36431/#review94460 --- Before we go too far down this path, can you please see https://reviews.apache.org/r/36424/ and see whether it does what you are trying to achieve here? (maybe not, I'm not familiar at all with `cli` - just asking) - Marco Massenzio On Aug. 4, 2015, 9:33 a.m., haosdent huang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36431/ > --- > > (Updated Aug. 4, 2015, 9:33 a.m.) > > > Review request for mesos, Adam B and Michael Park. > > > Bugs: MESOS-1084 > https://issues.apache.org/jira/browse/MESOS-1084 > > > Repository: mesos > > > Description > --- > > Show stdout/stderr when using mesos-execute. > > > Diffs > - > > src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc > > Diff: https://reviews.apache.org/r/36431/diff/ > > > Testing > --- > > make check > > > Thanks, > > haosdent huang > >
Re: Review Request 36431: Show stdout/stderr when using mesos-execute.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36431/#review91412 --- Patch looks great! Reviews applied: [36431] All tests passed. - Mesos ReviewBot On July 12, 2015, 4:31 p.m., haosdent huang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36431/ > --- > > (Updated July 12, 2015, 4:31 p.m.) > > > Review request for mesos and Adam B. > > > Bugs: MESOS-1084 > https://issues.apache.org/jira/browse/MESOS-1084 > > > Repository: mesos > > > Description > --- > > Show stdout/stderr when using mesos-execute. > > > Diffs > - > > src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc > > Diff: https://reviews.apache.org/r/36431/diff/ > > > Testing > --- > > make check > > > Thanks, > > haosdent huang > >
Re: Review Request 36431: Show stdout/stderr when using mesos-execute.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36431/ --- (Updated July 12, 2015, 4:31 p.m.) Review request for mesos and Adam B. Bugs: MESOS-1084 https://issues.apache.org/jira/browse/MESOS-1084 Repository: mesos Description --- Show stdout/stderr when using mesos-execute. Diffs (updated) - src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc Diff: https://reviews.apache.org/r/36431/diff/ Testing --- make check Thanks, haosdent huang
Re: Review Request 36431: Show stdout/stderr when using mesos-execute.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36431/ --- (Updated July 12, 2015, 4:30 p.m.) Review request for mesos and Adam B. Bugs: MESOS-1084 https://issues.apache.org/jira/browse/MESOS-1084 Repository: mesos Description --- Show stdout/stderr when using mesos-execute. Diffs (updated) - src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc Diff: https://reviews.apache.org/r/36431/diff/ Testing --- make check Thanks, haosdent huang
Re: Review Request 36431: Show stdout/stderr when using mesos-execute.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36431/#review91410 --- Patch looks great! Reviews applied: [36431] All tests passed. - Mesos ReviewBot On July 12, 2015, 10:57 a.m., haosdent huang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36431/ > --- > > (Updated July 12, 2015, 10:57 a.m.) > > > Review request for mesos and Adam B. > > > Bugs: MESOS-1084 > https://issues.apache.org/jira/browse/MESOS-1084 > > > Repository: mesos > > > Description > --- > > Show stdout/stderr when using mesos-execute. > > > Diffs > - > > src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc > > Diff: https://reviews.apache.org/r/36431/diff/ > > > Testing > --- > > make check > > > Thanks, > > haosdent huang > >
Re: Review Request 36431: Show stdout/stderr when using mesos-execute.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36431/ --- (Updated July 12, 2015, 10:57 a.m.) Review request for mesos and Adam B. Bugs: MESOS-1084 https://issues.apache.org/jira/browse/MESOS-1084 Repository: mesos Description --- Show stdout/stderr when using mesos-execute. Diffs (updated) - src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc Diff: https://reviews.apache.org/r/36431/diff/ Testing --- make check Thanks, haosdent huang
Re: Review Request 36431: Show stdout/stderr when using mesos-execute.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36431/#review91407 --- Patch looks great! Reviews applied: [36431] All tests passed. - Mesos ReviewBot On July 12, 2015, 9:20 a.m., haosdent huang wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36431/ > --- > > (Updated July 12, 2015, 9:20 a.m.) > > > Review request for mesos and Adam B. > > > Bugs: MESOS-1084 > https://issues.apache.org/jira/browse/MESOS-1084 > > > Repository: mesos > > > Description > --- > > Show stdout/stderr when using mesos-execute. > > > Diffs > - > > src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc > > Diff: https://reviews.apache.org/r/36431/diff/ > > > Testing > --- > > make check > > > Thanks, > > haosdent huang > >
Re: Review Request 36431: Show stdout/stderr when using mesos-execute.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36431/ --- (Updated July 12, 2015, 9:20 a.m.) Review request for mesos and Adam B. Bugs: MESOS-1084 https://issues.apache.org/jira/browse/MESOS-1084 Repository: mesos Description --- Show stdout/stderr when using mesos-execute. Diffs (updated) - src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc Diff: https://reviews.apache.org/r/36431/diff/ Testing --- make check Thanks, haosdent huang
Review Request 36431: Show stdout/stderr when using mesos-execute.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36431/ --- Review request for mesos and Adam B. Bugs: MESOS-1084 https://issues.apache.org/jira/browse/MESOS-1084 Repository: mesos Description --- Show stdout/stderr when using mesos-execute. Diffs - src/cli/execute.cpp d070164e080cb74ee15d3184487a121f429a29fc Diff: https://reviews.apache.org/r/36431/diff/ Testing --- make check Thanks, haosdent huang
