Re: Review Request 57596: Kept TaskInfo beyond first scheduler ack in command executor.

eviews.apache.org/r/57596/#review168976 --- On March 14, 2017, 2:08 p.m., Alexander Rukletsov wrote: > > --- > This is an automatically generated e-mail. To re

Review Request 57597: Added comments regarding exit status on Windows vs. Posix.

: https://reviews.apache.org/r/57597/diff/1/ Testing --- make check Thanks, Alexander Rukletsov

Re: Review Request 56213: Added check tests for command executor.

Diff: https://reviews.apache.org/r/56213/diff/6/ Changes: https://reviews.apache.org/r/56213/diff/5-6/ Testing --- See https://reviews.apache.org/r/56218/ Thanks, Alexander Rukletsov

Review Request 57598: Added comment regarding exit status on Windows vs. Posix in libprocess.

Description --- See summary. Diffs - 3rdparty/libprocess/include/process/subprocess_base.hpp 7117a2730ae8666d0b55cbe05162a0b4e843193c Diff: https://reviews.apache.org/r/57598/diff/1/ Testing --- make check Thanks, Alexander Rukletsov

Re: Review Request 56212: Added support for general checks to command executor.

: https://reviews.apache.org/r/56212/diff/5-6/ Testing --- See https://reviews.apache.org/r/56213/ Thanks, Alexander Rukletsov

Re: Review Request 57150: Simplified task id procurement in command executor.

://reviews.apache.org/r/56213/ Thanks, Alexander Rukletsov

Review Request 57596: Kept TaskInfo beyond first scheduler ack in command executor.

/ Thanks, Alexander Rukletsov

Re: Review Request 56210: Reused previous task status for health updates in command executor.

, Alexander Rukletsov

Re: Review Request 56211: Renamed health checker in command executor for clarity.

://reviews.apache.org/r/56211/diff/6/ Changes: https://reviews.apache.org/r/56211/diff/5-6/ Testing --- See https://reviews.apache.org/r/56213/ Thanks, Alexander Rukletsov

Re: Review Request 57149: Added a warning if command executor gets an unknown acknowledgement.

://reviews.apache.org/r/56213/ Thanks, Alexander Rukletsov

Re: Review Request 56208: Updated checks library with general check support.

> On March 1, 2017, 8:23 p.m., Vinod Kone wrote: > > src/checks/checker.cpp > > Line 424 (original), 440 (patched) > > <https://reviews.apache.org/r/56208/diff/5-7/?file=1628427#file1628427line452> > > > > why are we sending WEXITSTATUS and not ex

Re: Review Request 56208: Updated checks library with general check support.

/ Testing --- https://reviews.apache.org/r/56213/ Thanks, Alexander Rukletsov

Review Request 57594: Added a helper for building a task status from scratch.

0ed436faa68e984d0be4e5186138f738bc7f1b52 Diff: https://reviews.apache.org/r/57594/diff/1/ Testing --- make check Thanks, Alexander Rukletsov

Re: Review Request 57394: Overloaded `<<` for `CheckInfo::Type`.

/56213/ Thanks, Alexander Rukletsov

Re: Review Request 56016: Added a note about task status updates in scheduler and internal API.

508ff5960eb70b1299ef5ec02c23852c0088d295 Diff: https://reviews.apache.org/r/56016/diff/4/ Changes: https://reviews.apache.org/r/56016/diff/3-4/ Testing --- None: not a functional change. Thanks, Alexander Rukletsov

Re: Review Request 56017: Added a helper for building a task status from an existing one.

/ Changes: https://reviews.apache.org/r/56017/diff/3-4/ Testing --- make check Thanks, Alexander Rukletsov

Re: Review Request 57593: Sorted enum values in `authorizer.proto` by numerical order.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57593/#review168887 --- Ship it! Ship It! - Alexander Rukletsov On March 14, 2017

Re: Review Request 56208: Updated checks library with general check support.

> On March 1, 2017, 8:23 p.m., Vinod Kone wrote: > > src/checks/checker.cpp > > Line 424 (original), 440 (patched) > > <https://reviews.apache.org/r/56208/diff/5-7/?file=1628427#file1628427line452> > > > > why are we sending WEXITSTATUS and not ex

Re: Review Request 57472: Removing deprecated ACLs `SetQuota` and `RemoveQuota`.

/master/master.hpp Lines 1060-1064 (original), 1060-1064 (patched) <https://reviews.apache.org/r/57472/#comment241083> Kill the comment. - Alexander Rukletsov On March 13, 2017, 12:48 p.m., Alexander Rojas wrote: > >

Re: Review Request 57388: Added documentation for the 'RemoveNestedContainer' Agent API call.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57388/#review168804 --- Ship it! Ship It! - Alexander Rukletsov On March 13, 2017

Re: Review Request 57387: Implemented the Agent API call `RemoveNestedContainer`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57387/#review168803 --- Ship it! Ship It! - Alexander Rukletsov On March 13, 2017

Re: Review Request 57341: Made the usage of C++ namespaces in 'slave/http.cpp' consistent.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57341/#review168801 --- Ship it! Ship It! - Alexander Rukletsov On March 8, 2017

Re: Review Request 57386: Introduced changes to the authz protos needed for RemoveNestedContainer.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57386/#review168800 --- Ship it! Ship It! - Alexander Rukletsov On March 10, 2017

Re: Review Request 57385: Introduced proto changes needed for the RemoveNestedContainer API call.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57385/#review168798 --- Ship it! Ship It! - Alexander Rukletsov On March 13, 2017

Re: Review Request 57560: Removed unnecessary curly braces wrapping case statements.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57560/#review168799 --- Ship it! Ship It! - Alexander Rukletsov On March 13, 2017

Re: Review Request 57384: Added `Containerizer::remove`.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57384/#review168797 --- Ship it! Ship It! - Alexander Rukletsov On March 13, 2017

Re: Review Request 57387: Implemented the Agent API call `RemoveNestedContainer`.

> On March 13, 2017, 12:05 p.m., Alexander Rukletsov wrote: > > src/authorizer/local/authorizer.cpp > > Lines 1005-1017 (patched) > > <https://reviews.apache.org/r/57387/diff/3/?file=1661472#file1661472line1005> > > > > Please do not enclose t

Re: Review Request 57388: Added documentation for the 'RemoveNestedContainer' Agent API call.

tps://reviews.apache.org/r/57388/#comment241012> Ups? - Alexander Rukletsov On March 10, 2017, 6:05 p.m., Gastón Kleiman wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.

Re: Review Request 57387: Implemented the Agent API call `RemoveNestedContainer`.

) <https://reviews.apache.org/r/57387/#comment241010> Please do not enclose the `case` block in `{}` for consistency. src/tests/authorization_tests.cpp Lines 3825 (patched) <https://reviews.apache.org/r/57387/#comment241011> s/deleting/removing - Alexander Rukletsov On March

Re: Review Request 57384: Added `Containerizer::cleanupArtifacts`.

--- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57384/ > ------- > > (Updated March 9, 2017, 1:46 p.m.) > > > Review request for mesos, Alexander Rukletsov, Jie Yu, Kevin Klues, and Vinod > Kone. > > > Bugs: ME

Re: Review Request 56213: Added check tests for command executor.

Diff: https://reviews.apache.org/r/56213/diff/5/ Changes: https://reviews.apache.org/r/56213/diff/4-5/ Testing --- See https://reviews.apache.org/r/56218/ Thanks, Alexander Rukletsov

Re: Review Request 56212: Added support for general checks to command executor.

: https://reviews.apache.org/r/56212/diff/4-5/ Testing --- See https://reviews.apache.org/r/56213/ Thanks, Alexander Rukletsov

Re: Review Request 57150: Simplified task id procurement in command executor.

://reviews.apache.org/r/56213/ Thanks, Alexander Rukletsov

Re: Review Request 56211: Renamed health checker in command executor for clarity.

://reviews.apache.org/r/56211/diff/5/ Changes: https://reviews.apache.org/r/56211/diff/4-5/ Testing --- See https://reviews.apache.org/r/56213/ Thanks, Alexander Rukletsov

Re: Review Request 56210: Reused previous task status for health updates in command executor.

, Alexander Rukletsov

Re: Review Request 57149: Added a warning if command executor gets an unknown acknowledgement.

://reviews.apache.org/r/56213/ Thanks, Alexander Rukletsov

Re: Review Request 56208: Updated checks library with general check support.

/ Testing --- https://reviews.apache.org/r/56213/ Thanks, Alexander Rukletsov

Review Request 57394: Overloaded `<<` for `CheckInfo::Type`.

d86d56d4e1d353d6e82ccff89357bf2abec6eded src/v1/mesos.cpp 5b89603e1b760256c584a188e93be31eeaea7ce2 Diff: https://reviews.apache.org/r/57394/diff/1/ Testing --- https://reviews.apache.org/r/56213/ Thanks, Alexander Rukletsov

Re: Review Request 56016: Added a note about task status updates in scheduler and internal API.

508ff5960eb70b1299ef5ec02c23852c0088d295 Diff: https://reviews.apache.org/r/56016/diff/3/ Changes: https://reviews.apache.org/r/56016/diff/2-3/ Testing --- None: not a functional change. Thanks, Alexander Rukletsov

Re: Review Request 56017: Added a helper for building a task status from an existing one.

/ Changes: https://reviews.apache.org/r/56017/diff/2-3/ Testing --- make check Thanks, Alexander Rukletsov

Re: Review Request 56212: Added support for general checks to command executor.

scription > --- > > See summary. > > > Diffs > - > > src/launcher/executor.cpp adcc50fb7df91b9f8dff0f583b61e0efcd6da378 > > > Diff: https://reviews.apache.org/r/56212/diff/4/ > > > Testing > --- > > See https://reviews.apache.org/r/56213/ > > > Thanks, > > Alexander Rukletsov > >

Re: Review Request 56208: Updated checks library with general check support.

eviews.apache.org/r/56208/#review167557 --- On March 1, 2017, 12:50 p.m., Alexander Rukletsov wrote: > > --- > This is an automatically generated e-mail. To re

Re: Review Request 56017: Added a helper for building a task status from an existing one.

nd `TaskStatus` go out of sync. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56017/#review167556 ------- On Feb. 2

Re: Review Request 56208: Updated checks library with general check support.

/7/ Changes: https://reviews.apache.org/r/56208/diff/6-7/ Testing --- https://reviews.apache.org/r/56213/ Thanks, Alexander Rukletsov

Re: Review Request 56213: Added check tests for command executor.

mail. To reply, visit: https://reviews.apache.org/r/56213/#review165049 --- On Feb. 28, 2017, 3:55 p.m., Alexander Rukletsov wrote: > > --- > This is an automatically generated e

Re: Review Request 56213: Added check tests for command executor.

tests/check_tests.cpp > > Lines 391 (patched) > > <https://reviews.apache.org/r/56213/diff/3/?file=1627707#file1627707line391> > > > > s/checkCommandString/command/ To avoid the aforementioned ambiguity, I'd go with `checkCommand`. - Alexander

Re: Review Request 56213: Added check tests for command executor.

e) 2) repeat checking initial check status (done in `CommandCheckDeliveredAndReconciled`). - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56213/#review165568 ---

Re: Review Request 56213: Added check tests for command executor.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56213/#review165724 --- On Feb. 28, 2017, 3:55 p.m., Alexander Rukletsov wrote: > > ---

Re: Review Request 56208: Updated checks library with general check support.

/ Thanks, Alexander Rukletsov

Review Request 57150: Simplified task id procurement in command executor.

Description --- See summary. Diffs - src/launcher/executor.cpp adcc50fb7df91b9f8dff0f583b61e0efcd6da378 Diff: https://reviews.apache.org/r/57150/diff/ Testing --- See https://reviews.apache.org/r/56213/ Thanks, Alexander Rukletsov

Re: Review Request 56213: Added check tests for command executor.

: https://reviews.apache.org/r/56213/diff/ Testing --- See https://reviews.apache.org/r/56218/ Thanks, Alexander Rukletsov

Re: Review Request 56212: Added support for general checks to command executor.

(updated) --- See https://reviews.apache.org/r/56213/ Thanks, Alexander Rukletsov

Re: Review Request 56211: Renamed health checker in command executor for clarity.

://reviews.apache.org/r/56211/diff/ Testing (updated) --- See https://reviews.apache.org/r/56213/ Thanks, Alexander Rukletsov

Re: Review Request 56211: Renamed health checker in command executor for clarity.

: https://reviews.apache.org/r/56211/diff/ Testing --- See https://reviews.apache.org/r/56218/ Thanks, Alexander Rukletsov

Re: Review Request 56210: Reused previous task status for health updates in command executor.

(updated) --- See https://reviews.apache.org/r/56213/ Thanks, Alexander Rukletsov

Review Request 57149: Added a warning if command executor gets an unknown acknowledgement.

Description --- See summary. Diffs - src/launcher/executor.cpp adcc50fb7df91b9f8dff0f583b61e0efcd6da378 Diff: https://reviews.apache.org/r/57149/diff/ Testing --- https://reviews.apache.org/r/56213/ Thanks, Alexander Rukletsov

Re: Review Request 56208: Updated checks library with general check support.

/ Thanks, Alexander Rukletsov

Re: Review Request 56017: Added a helper for building a task status from an existing one.

/protobuf_utils.cpp 944263bbaa87a65005fd924bccfadb7293312fa0 Diff: https://reviews.apache.org/r/56017/diff/ Testing --- make check Thanks, Alexander Rukletsov

Re: Review Request 56208: Updated checks library with general check support.

t; might have multiple checkers running at the same time. > > Alexander Rukletsov wrote: > To do this we have to cache `taskId` in the class. Is it worth it? > > Gastón Kleiman wrote: > As per offline discussion, I think that we could move this logging > s

Re: Review Request 56212: Added support for general checks to command executor.

erminal states means changing current behaviour. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56212/#review165550 ------- On Feb. 8, 2017, 4:56 p.m., Alexander Rukletsov wrote: > > -

Re: Review Request 56448: Shortened continuation name in default executor.

, Alexander Rukletsov

Re: Review Request 56449: Moved health checker closer to container in default executor.

- src/launcher/default_executor.cpp e63cf153831088851863d0956455a024e9bc172a Diff: https://reviews.apache.org/r/56449/diff/ Testing --- See https://reviews.apache.org/r/56218/ Thanks, Alexander Rukletsov

Re: Review Request 56210: Reused previous task status to generate a new one in command executor.

--- See https://reviews.apache.org/r/56218/ Thanks, Alexander Rukletsov

Re: Review Request 56209: Hashed unacknowledged updates by UUID string in command executor.

/56218/ Thanks, Alexander Rukletsov

Re: Review Request 56208: Updated checks library with general check support.

eviews.apache.org/r/56208/#review165521 --- On Feb. 28, 2017, 11:12 a.m., Alexander Rukletsov wrote: > > --- > This is an automatically generated e-mail. To re

Re: Review Request 56208: Updated checks library with general check support.

> > > Is that idiom preferred? Good idea. Fixed. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56208/#review165521 -------

Re: Review Request 56208: Updated checks library with general check support.

, Alexander Rukletsov

Re: Review Request 56742: Updated CHANGELOG for Mesos 1.2.0 release.

> On Feb. 21, 2017, 7:11 p.m., Vinod Kone wrote: > > CHANGELOG, line 111 > > > > > > I think we can graduate this? cc @alexR We haven't tested it on windows yet. Also we may be changing the protos in the nearest

Review Request 56551: Updated CHANGELOG with still experimental features from 1.1.0.

--- See summary. Diffs - CHANGELOG 1478ad4dd3b20fc768fd679f81d803acf0995ad2 Diff: https://reviews.apache.org/r/56551/diff/ Testing --- None: not a functional change. Thanks, Alexander Rukletsov

Re: Review Request 55901: Added support for command health checks to the default executor.

ws.apache.org/r/55901/#comment236770> What do you get from the agent call: status or exit code directly? It's probably not that important here, but it is for checks. - Alexander Rukletsov On Feb. 8, 2017, 1:27 p.m., Gastón Kleiman wrote: > > ---

Re: Review Request 56208: Updated checks library with general check support.

/ Thanks, Alexander Rukletsov

Re: Review Request 56210: Reused previous task status to generate a new one in command executor.

/ Testing --- See https://reviews.apache.org/r/56218/ Thanks, Alexander Rukletsov

Re: Review Request 56218: Added some check tests for default executor.

--- Tested the complete chain starting at https://reviews.apache.org/r/56208/ make check Thanks, Alexander Rukletsov

Re: Review Request 56210: Reused previous task status to generate a new one in command executor.

econciliation. For > instance, consider a check status being sent; in this status update > we must include the latest known health information. > > > Diffs > - > > src/launcher/executor.cpp d9417ce1d5b108f5292a66010eab80f11552a969 > > Diff: https://reviews.apache.org/r/56210/diff/ > > > Testing > --- > > See https://reviews.apache.org/r/56218/ > > > Thanks, > > Alexander Rukletsov > >

Re: Review Request 56212: Added support for general checks to command executor.

-- > > See summary. > > > Diffs > - > > src/launcher/executor.cpp d9417ce1d5b108f5292a66010eab80f11552a969 > > Diff: https://reviews.apache.org/r/56212/diff/ > > > Testing > --- > > See https://reviews.apache.org/r/56218/ > > > Thanks, > > Alexander Rukletsov > >

Re: Review Request 56210: Reused previous task status to generate a new one in command executor.

ditto. no `reason` argument in this review. See above. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56210/#review164584 -------

Re: Review Request 56208: Updated checks library with general check support.

https://reviews.apache.org/r/56218/ Thanks, Alexander Rukletsov

Re: Review Request 56215: Reused previous task status to generate a new one in default executor.

: https://reviews.apache.org/r/56215/diff/ Testing --- See https://reviews.apache.org/r/56218/ Thanks, Alexander Rukletsov

Re: Review Request 55901: Added support for command health checks to the default executor.

e base, transform the future not really to add extra logging > information. > > Alexander Rukletsov wrote: > We use `.repair` for adjusting the message or changing the error type, > which is technically the same. Here are some examples from the code: > > https://github.com/a

Re: Review Request 56211: Renamed health checker in command executor for clarity.

--- See https://reviews.apache.org/r/56218/ Thanks, Alexander Rukletsov

Re: Review Request 56214: Hashed unacknowledged updates by UUID string in default executor.

://reviews.apache.org/r/56218/ Thanks, Alexander Rukletsov

Re: Review Request 56448: Shortened continuation name in default executor.

/ Thanks, Alexander Rukletsov

Re: Review Request 56447: Added a helper function for creating valid empty `CheckStatusInfo`s.

/56218/ Thanks, Alexander Rukletsov

Re: Review Request 56209: Hashed unacknowledged updates by UUID string in command executor.

://reviews.apache.org/r/56218/ Thanks, Alexander Rukletsov

Re: Review Request 56215: Reused previous task status to generate a new one in default executor.

/ Testing --- See https://reviews.apache.org/r/56218/ Thanks, Alexander Rukletsov

Re: Review Request 56217: Added support for general checks to default executor.

/ Testing --- See https://reviews.apache.org/r/56218/ Thanks, Alexander Rukletsov

Re: Review Request 56218: Added some check tests for default executor.

--- Tested the complete chain starting at https://reviews.apache.org/r/56208/ make check Thanks, Alexander Rukletsov

Re: Review Request 56213: Added check tests for command executor.

Diff: https://reviews.apache.org/r/56213/diff/ Testing --- See https://reviews.apache.org/r/56218/ Thanks, Alexander Rukletsov

Re: Review Request 56449: Moved health checker closer to container in default executor.

) - src/launcher/default_executor.cpp e63cf153831088851863d0956455a024e9bc172a Diff: https://reviews.apache.org/r/56449/diff/ Testing --- See https://reviews.apache.org/r/56218/ Thanks, Alexander Rukletsov

Re: Review Request 56208: Updated checks library with general check support.

/ Thanks, Alexander Rukletsov

Re: Review Request 56210: Reused previous task status to generate a new one in command executor.

/ Testing --- See https://reviews.apache.org/r/56218/ Thanks, Alexander Rukletsov

Re: Review Request 56212: Added support for general checks to command executor.

--- See https://reviews.apache.org/r/56218/ Thanks, Alexander Rukletsov

Re: Review Request 56208: Updated checks library with general check support.

ed e-mail. To reply, visit: https://reviews.apache.org/r/56208/#review164424 --- On Feb. 8, 2017, 3:13 p.m., Alexander Rukletsov wrote: > > --- > This is an automatically

Re: Review Request 56448: Shortened continuation name in default executor.

/r/56448/diff/ Testing --- See https://reviews.apache.org/r/56218/ Thanks, Alexander Rukletsov

Re: Review Request 56217: Added support for general checks to default executor.

/ Testing --- See https://reviews.apache.org/r/56218/ Thanks, Alexander Rukletsov

Re: Review Request 56218: Added some check tests for default executor.

--- Tested the complete chain starting at https://reviews.apache.org/r/56208/ make check Thanks, Alexander Rukletsov

Re: Review Request 56215: Reused previous task status to generate a new one in default executor.

/ Testing --- See https://reviews.apache.org/r/56218/ Thanks, Alexander Rukletsov

Re: Review Request 56214: Hashed unacknowledged updates by UUID string in default executor.

--- See https://reviews.apache.org/r/56218/ Thanks, Alexander Rukletsov

Review Request 56449: Moved health checker closer to container in default executor.

e63cf153831088851863d0956455a024e9bc172a Diff: https://reviews.apache.org/r/56449/diff/ Testing --- See https://reviews.apache.org/r/56218/ Thanks, Alexander Rukletsov

Review Request 56448: Augmented continuation name in default executor.

Description --- See summary. Diffs - src/launcher/default_executor.cpp e63cf153831088851863d0956455a024e9bc172a Diff: https://reviews.apache.org/r/56448/diff/ Testing --- See https://reviews.apache.org/r/56218/ Thanks, Alexander Rukletsov

<    5   6   7   8   9   10   11   12   13   14   >