Re: Review Request 61723: Bumped version to 1.5.0.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61723/#review183179 --- Ship it! Ship It! - Adam B On Aug. 17, 2017, 4:41 p.m

Re: Review Request 60426: Marked 1.2.2 as WIP in CHANGELOG.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60426/#review178900 --- Ship it! Ship It! - Adam B On June 26, 2017, 3:42 a.m

Re: Review Request 60405: Documented resource format in agent <-> master protocol.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60405/#review178847 --- Ship it! Ship It! - Adam B On June 24, 2017, 10:30 a.m

Re: Review Request 60405: Documented resource format in agent <-> master protocol.

> On June 23, 2017, 7:21 p.m., Adam B wrote: > > src/slave/slave.cpp > > Lines 1405-1407 (original), 1405-1408 (patched) > > <https://reviews.apache.org/r/60405/diff/2/?file=1761692#file1761692line1405> > > > > If this agent has refinements, and we

Re: Review Request 60407: Avoided master crash on agent re-registration.

> On June 23, 2017, 7:35 p.m., Adam B wrote: > > What's the JIRA for this bug? - Adam --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60407/#rev

Re: Review Request 60407: Avoided master crash on agent re-registration.

) <https://reviews.apache.org/r/60407/#comment253088> Even if we can't `validateDynamicReservationInfo` (or `validateDiskInfo`?), would it be worthwhile to `validateGpus`? Maybe clone/parameterize `resource::validate` to validate what we can? - Adam B On June 23, 2017, 6:48 p.m.

Re: Review Request 60406: Added sanity check to resource downgrade on agent registration.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60406/#review178835 --- Ship it! Ship It! - Adam B On June 23, 2017, 6:48 p.m

Re: Review Request 60405: Documented resource format in agent <-> master protocol.

r something in between? src/slave/slave.cpp Line 1408 (original), 1412-1414 (patched) <https://reviews.apache.org/r/60405/#comment253087> We could at least log an INFO/WARN if we aren't able to downgrade, and still send it anyway. - Adam B On June 23, 2017, 6:48 p.m.,

Re: Review Request 60404: Documented the content of the `SlaveInfo.resources` field.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60404/#review178833 --- Ship it! Ship It! - Adam B On June 23, 2017, 6:49 p.m

Re: Review Request 58964: Added authorization support for operator endpoints.

atched) <https://reviews.apache.org/r/58964/#comment248669> These should probably be alpha-sorted as well src/authorizer/local/authorizer.cpp Lines 1024-1025 (original), 1042-1048 (patched) <https://reviews.apache.org/r/58964/#comment248670> These should probably b

Re: Review Request 58955: Adds authorization for the master's v1 API call SET_LOGGING_LEVEL.

e { Try approved = approver->approved((ObjectApprover::Object())); ``` src/tests/api_tests.cpp Lines 772 (patched) <https://reviews.apache.org/r/58955/#comment248657> Is there a v0 equivalent of this test for `/logging/toggle`? - Adam B On May 12, 2017, 2:28

Re: Review Request 58099: Added authorization for frameworks in `GetRoles` v1 API.

clone of `roles()`. Can you find a way to reduce the redundance? - Adam B On May 10, 2017, 6:52 a.m., Jay Guo wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 58097: Added a test to check framework filtering in /roles endpoint.

g/r/58097/#comment248648> Please validate the single role name is as expected. - Adam B On May 10, 2017, 6:47 a.m., Jay Guo wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://re

Re: Review Request 58096: Added authorization for frameworks in /roles endpoint.

s, not an approver per framework. src/master/http.cpp Lines 3524 (patched) <https://reviews.apache.org/r/58096/#comment248644> `futures` is an over-vague variable name, especially since neither are Futures by this point. Can we do better? - Adam B On May 10, 20

Re: Review Request 58095: Refactored `Master::Http::roles` to use `jsonify`.

y rename `filteredRoles` to `activeRoles`? That variable is the result of the `_roles(principal)` authorization filtering. Maybe `authorizedRoles` if you're going to rename. Or call them `roleNames` since it's just the string representation and not the full `Role` object. - Adam B O

Re: Review Request 58095: Refactored `Master::Http::roles` to use `jsonify`.

lly generated e-mail. To reply, visit: > https://reviews.apache.org/r/58095/ > --- > > (Updated May 10, 2017, 9:32 a.m.) > > > Review request for mesos, Adam B, Alexander Rojas, Benjamin Mahler, and > Michael Park. > &

Re: Review Request 59198: Documented that Mesos 1.3.0, 1.2.1 don't allow old agents to register.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59198/#review174730 --- Ship it! Ship It! - Adam B On May 11, 2017, 2:10 p.m., Neil

Re: Review Request 59198: Documented that Mesos 1.3.0, 1.2.1 don't allow old agents to register.

247940> Ditto on `> 1.0` and `pre-1.0`. docs/upgrades.md Lines 50 (patched) <https://reviews.apache.org/r/59198/#comment247941> Should we add the same thing to 1.2.x (with a special note that it only applies to `1.2.1+`)? - Adam B On May 11, 2017,

Re: Review Request 59198: Documented that Mesos 1.3.0, 1.2.1 don't allow old agents to register.

s 300 (patched) <https://reviews.apache.org/r/59198/#comment247928> s/1.3.0/1.2.1/ ? - Adam B On May 11, 2017, 11:58 a.m., Neil Conway wrote: > > --- > This is an automatically generated e-mail. To reply, visit:

Re: Review Request 58942: CHANGELOG for 1.3.0 release.

942/#comment246939> I bet we can graduate a few more of these. Maybe not the ones that still have "Unresolved Critical Issues", but some of the others. - Adam B On May 2, 2017, 4:48 p.m., Mi

Re: Review Request 58942: CHANGELOG for 1.3.0 release.

> On May 3, 2017, 4:48 a.m., Stephan Erb wrote: > > CHANGELOG > > Lines 117 (patched) > > > > > > I was once confused by this "All issues" title in presence of the > > "Unresolved Critical Issues". > > > >

Re: Review Request 58923: Added new ContainerLaunchInfo task_environment.

numeric and put it at the end, or put it next to `environment` so you can explain the difference. - Adam B On May 2, 2017, 10:28 a.m., Till Toenshoff wrote: > > --- > This is an automatically generated e-mail. To rep

Re: Review Request 57474: Added test for authorization of hierarchical roles.

commit. src/tests/authorization_tests.cpp Line 1493 (original), 1493 (patched) <https://reviews.apache.org/r/57474/#comment246237> s/crete/create/ - Adam B On April 18, 2017, 6:28 a.m., Alexander Rojas

Re: Review Request 57473: Added support for authorization of Hierachical roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57473/#review173166 --- Ship it! Ship It! - Adam B On April 11, 2017, 3:58 a.m

Re: Review Request 58690: Fixed leak of sensitive data in agent log of docker containerizer.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58690/#review172881 --- Ship it! Ship It! - Adam B On April 24, 2017, 5:41 p.m

Re: Review Request 57474: Added test for authorization of hierarchical roles.

c/tests/authorization_tests.cpp Lines 4760 (patched) <https://reviews.apache.org/r/57474/#comment244717> s/view any role/view quota for any role/ - Adam B On April 11, 2017, 1:47 a.m., Alexander Rojas wrote: > > -

Re: Review Request 57474: Added test for authorization of hierarchical roles.

April 11, 2017, 1:47 a.m., Alexander Rojas wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57474/ > --- > > (Upda

Re: Review Request 58253: Added a ContainerID to 'ObjectApprover::Object'.

trying to authorize. Can you elaborate on what (new) action will use this object type, and how, and from where? - Adam B On April 6, 2017, 8:33 p.m., Greg Mann wrote: > > --- > This is an automatically generated e-mail. To reply,

Re: Review Request 57788: Changed allocator to skip allocation on weight and quota changes.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57788/#review171538 --- Ship it! Ship It! - Adam B On March 22, 2017, 11:36 a.m

Re: Review Request 57730: Fixed example tests which broke due to the new `register_agents` ACL.

tests that don't need it? - Adam B On March 17, 2017, 8:53 a.m., Jiang Yan Xu wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.

Re: Review Request 57710: Added `register_agents` to authorization.md.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57710/#review171535 --- Ship it! Ship It! - Adam B On March 16, 2017, 4:07 p.m

Re: Review Request 57534: Added and implemented RegisterAgent ACL.

/authorizer/local/authorizer.cpp Lines 1062 (patched) <https://reviews.apache.org/r/57534/#comment244480> Unnecessary `break` after `return` (recently removed elsewhere) - Adam B On March 28, 2017, 1:24 a.m., Jiang Yan Xu

Re: Review Request 58304: Remove unnecessary hashmap lookups.

/master/master.cpp Lines 6429-6431 (original), 6433-6435 (patched) <https://reviews.apache.org/r/58304/#comment244479> s/shutdown.executor_id()/executorId/ s/shutdown.slave_id()/slaveId/ (now that you have them already) - Adam B On April 10, 2017, 8:50 a.m., James Peach

Re: Review Request 58303: Pass the slave pointer directly to Master::removeTask.

Lines 8435-8439 (original), 8435-8436 (patched) <https://reviews.apache.org/r/58303/#comment244478> Nit: I'd probably swap these CHECKs to match the parameter order (or swap the parameters). - Adam B On April 10, 2017, 8:48 a.m., James Peach

Re: Review Request 57652: Allow authenticators to return any http Response.

struct, and b) teach any users of the http authenticator (i.e. master, agent, etc.) to handle/log such a result and return the appropriate response. - Adam B On April 5, 2017, 12:13 p.m., Silas Snider wrote: > > --- > This is a

Re: Review Request 57989: Removing deprecated ACL `ShutdownFramework`.

placed. - Adam B On March 28, 2017, 2:35 a.m., Alexander Rojas wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.

Re: Review Request 58252: Allowed the local authorizer to accept subjects with no value.

_value() || request.subject().has_claims()); ``` - Adam B On April 7, 2017, 4:15 p.m., Greg Mann wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.

Re: Review Request 57473: Added support for authorization of Hierachical roles.

src/authorizer/local/authorizer.cpp Lines 677 (patched) <https://reviews.apache.org/r/57473/#comment244474> Sounds like reason for an assert, not a mere comment. - Adam B On April 10, 2017, 3:

Re: Review Request 58292: Removed unnecesary break statements in local approver.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58292/#review171511 --- Ship it! Ship It! - Adam B On April 10, 2017, 3:09 a.m

Re: Review Request 57166: Updated role validation for hierarchical roles.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57166/#review171391 --- Ship it! Ship It! - Adam B On March 7, 2017, 7:38 a.m

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

. CHANGELOG Lines 7-8 (patched) <https://reviews.apache.org/r/57472/#comment243071> This change is only applicable to the local authorizer, right? Other authorizers probably don't use `--acls`. - Adam B On March 21, 2017, 6:28 a.m., Alexander Rojas

Re: Review Request 57846: Docker environment gets passed on docker run command.

suggests. - Adam B On March 22, 2017, 12:57 p.m., Till Toenshoff wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 57762: Fixed environment duplication in command executor.

r/executor.cpp Lines 507 (patched) <https://reviews.apache.org/r/57762/#comment242678> "Overwriting environment variable 'foo' with value from command environment." - Adam B On March 23, 2017, 7:24 p.m., Till Toenshoff wrote: > >

Re: Review Request 57889: Fixed flags logging in Docker executor.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57889/#review169923 --- Ship it! Ship It! - Adam B On March 23, 2017, 12:43 p.m

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

able as well. - Adam B On March 10, 2017, 12:43 a.m., Alexander Rojas wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://re

Review Request 57442: Update website for Mesos 1.2.0 release.

site/source/blog/2017-03-08-mesos-1-2-0-released.md PRE-CREATION site/source/layouts/post.erb fdb566b01892d87fe6e121c958fe07b29cedf072 Diff: https://reviews.apache.org/r/57442/diff/1/ Testing --- Thanks, Adam B

Re: Review Request 56805: Simplified interface for setting weights in allocator.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56805/#review168076 --- Ship it! Ship It! - Adam B On Feb. 28, 2017, 12:24 p.m

Re: Review Request 56623: Implemented the 'Principal' type in libprocess.

(original), 108 (patched) <https://reviews.apache.org/r/56623/#comment239791> "authentication principal"? - Adam B On Feb. 28, 2017, 10:28 a.m., Greg Mann wrote: > > --- > This is an automatically generate

Re: Review Request 57054: Fixed a bug in master and agent handler authorization logic.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57054/#review167660 --- Ship it! Ship It! - Adam B On Feb. 27, 2017, 9:31 p.m

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

> On Feb. 23, 2017, 4:39 p.m., Benjamin Bannier wrote: > > We simultanously use _unified containerizer_ and _Mesos containerizer_ here > > to describe 1.2.0 changes. I think picking a single name, at least in > > high-level descriptions, might be less confusing. > >

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

Kone. Changes --- Mesos containerizer, rlimit. Repository: mesos Description --- Updated CHANGELOG for Mesos 1.2.0 release. Diffs (updated) - CHANGELOG 1478ad4dd3b20fc768fd679f81d803acf0995ad2 Diff: https://reviews.apache.org/r/56742/diff/ Testing --- Thanks, Adam

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56742/#review166621 --- On Feb. 23, 2017, 4:04 p.m., Adam B wrote: > > --

Re: Review Request 56867: Added upgrade guide for 1.2.x.

--- Use proper authorization action names/cases Repository: mesos Description --- Added upgrade guide for 1.2.x. Diffs (updated) - docs/upgrades.md 74f4367020d7025bdcfbbf50a166a644ee5050bc Diff: https://reviews.apache.org/r/56867/diff/ Testing --- Thanks, Adam B

Re: Review Request 56867: Added upgrade guide for 1.2.x.

--- New/Added is 'A' Repository: mesos Description --- Added upgrade guide for 1.2.x. Diffs (updated) - docs/upgrades.md 74f4367020d7025bdcfbbf50a166a644ee5050bc Diff: https://reviews.apache.org/r/56867/diff/ Testing --- Thanks, Adam B

Re: Review Request 56867: Added upgrade guide for 1.2.x.

(updated) - Added upgrade guide for 1.2.x. Repository: mesos Description --- Added upgrade guide for 1.2.x. Diffs (updated) - docs/upgrades.md 74f4367020d7025bdcfbbf50a166a644ee5050bc Diff: https://reviews.apache.org/r/56867/diff/ Testing --- Thanks, Adam B

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

can wait. It stays experimental then. - Adam --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56742/#review166203 ------- On Feb. 23, 2017, 4:04 p.m., Adam B wrote: > >

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

Kone. Repository: mesos Description --- Updated CHANGELOG for Mesos 1.2.0 release. Diffs (updated) - CHANGELOG 1478ad4dd3b20fc768fd679f81d803acf0995ad2 Diff: https://reviews.apache.org/r/56742/diff/ Testing --- Thanks, Adam B

Review Request 57005: Updated configuration.md for --http_heartbeat_interval.

--- Updated configuration.md for --http_heartbeat_interval. Diffs - docs/configuration.md b3e1f8ec20c663a49128a858f636580983891c8b Diff: https://reviews.apache.org/r/57005/diff/ Testing --- Thanks, Adam B

Re: Review Request 56711: Fixed fetcher to not pick up environment variables it should not see.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56711/#review166456 --- Ship it! Ship It! - Adam B On Feb. 22, 2017, 4:41 p.m

Re: Review Request 56711: Fixed fetcher to not pick up environment variables it should not see.

en you could fit these two one one line. - Adam B On Feb. 21, 2017, 3:46 p.m., Till Toenshoff wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://re

Re: Review Request 56771: Added regression test against fetcher SSL spillover.

> On Feb. 21, 2017, 5:16 p.m., Adam B wrote: > > Looks good to me. Just a cuminor comments. s/cuminor/few minor/ - Adam --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56771/#rev

Re: Review Request 56771: Added regression test against fetcher SSL spillover.

is the final check. Please add a comment explaining how a successful fetch means a successful test run. - Adam B On Feb. 21, 2017, 7:56 a.m., Till Toenshoff wrote: > > --- > This is an automatically generated e-mail. To

Re: Review Request 56867: WIP: Added upgrade guide for 1.2.x.

----- On Feb. 20, 2017, 11:57 p.m., Adam B wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56867/ > -

Re: Review Request 56711: Fixed fetcher to not pick up environment variables it should not see.

tps://reviews.apache.org/r/56711/#comment238062> Why not `whitelist.contains(toUpper(key))`? - Adam B On Feb. 21, 2017, 8:04 a.m., Till Toenshoff wrote: > > --- > This is an automatically generated e-mail. To reply,

Review Request 56867: WIP: Added upgrade guide for 1.2.x.

upgrade guide for 1.2.x. Diffs - docs/upgrades.md 74f4367020d7025bdcfbbf50a166a644ee5050bc Diff: https://reviews.apache.org/r/56867/diff/ Testing --- Thanks, Adam B

Re: Review Request 56812: Updated agent handlers to use 'AuthenticationContext'.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56812/#review166123 --- Ship it! Ship It! - Adam B On Feb. 17, 2017, 7:02 p.m

Re: Review Request 56804: Cleaned up weights handling code.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56804/#review166122 --- Ship it! Ship It! - Adam B On Feb. 17, 2017, 4:14 p.m

Re: Review Request 54784: Added authorization tests when trying to attach to a container input.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54784/#review166121 --- Ship it! Ship It! - Adam B On Feb. 15, 2017, 6:31 a.m

Re: Review Request 54783: Adds authorization tests of launching container sessions API.

. Authorized access is tested by `AgentAPITest.LaunchNestedContainerSession` - Adam B On Feb. 15, 2017, 6:30 a.m., Alexander Rojas wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Re: Review Request 56619: Updated 'Files' handlers to use 'AuthenticationContext'.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56619/#review166119 --- Ship it! Ship It! - Adam B On Feb. 17, 2017, 7 p.m., Greg

Re: Review Request 56618: Updated common Mesos code to use 'AuthenticationContext'.

- 30) <https://reviews.apache.org/r/56618/#comment238013> "currently only a value" is no longer accurate - Adam B On Feb. 17, 2017, 2:34 p.m., Greg Mann wrote: > > --- > This is an automatically generate

Re: Review Request 56617: Updated libprocess handlers to use 'AuthenticationContext'.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56617/#review166117 --- Ship it! Ship It! - Adam B On Feb. 13, 2017, 7:41 p.m

Re: Review Request 56623: Implemented 'AuthenticationContext' in libprocess.

of the patch chain yet 3rdparty/libprocess/include/process/authenticator.hpp (line 36) <https://reviews.apache.org/r/56623/#comment238012> "claims" or "labels"? I kinda like "claims" - Adam B On Feb.

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

Kone. Changes --- Updates! Repository: mesos Description --- Updated CHANGELOG for Mesos 1.2.0 release. Diffs (updated) - CHANGELOG 1478ad4dd3b20fc768fd679f81d803acf0995ad2 Diff: https://reviews.apache.org/r/56742/diff/ Testing --- Thanks, Adam B

Re: Review Request 56711: Fixed fetcher to not pick up environment variables it should not see.

> On Feb. 16, 2017, 1:39 a.m., Adam B wrote: > > src/slave/containerizer/fetcher.cpp, lines 827-828 > > <https://reviews.apache.org/r/56711/diff/1/?file=1634532#file1634532line827> > > > > How confident are you that the fetcher doesn't actually need any o

Review Request 56742: Updated CHANGELOG for Mesos 1.2.0 release.

CHANGELOG for Mesos 1.2.0 release. Diffs - CHANGELOG 1478ad4dd3b20fc768fd679f81d803acf0995ad2 Diff: https://reviews.apache.org/r/56742/diff/ Testing --- Thanks, Adam B

Re: Review Request 56711: Fixed fetcher to not pick up environment variables it should not see.

tps://reviews.apache.org/r/56711/#comment237661> How confident are you that the fetcher doesn't actually need any of these env vars? How can we test/prove it? - Adam B On Feb. 15, 2017, 5:31 p.m., Till Toenshoff

Re: Review Request 56178: Enabled the authorizer to work with MULTI_ROLE frameworks.

--- > > (Updated Feb. 9, 2017, 1:26 a.m.) > > > Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler. > > > Bugs: MESOS-7022 > https://issues.apache.org/jira/browse/MESOS-7022 > > > Repository: mesos >

Re: Review Request 56053: Added a 'SECRET' type to the 'Environment' protobuf message.

, and there's only a single test for the REFERENCE type. Is there nothing else we should test there? - Adam B On Feb. 8, 2017, 10:34 p.m., Greg Mann wrote: > > --- > This is an automatically generated e-mail. To reply, visit

Re: Review Request 56052: Added the 'Secret' protobuf message.

roto3? I can't imagine any use for a Value secret without a value. Optional could still be useful if we eventually decide to replace `string text` with a different type - Adam B On Feb. 8, 2017, 10:33 p.m., Gre

Re: Review Request 56178: Enabled the authorizer to work with MULTI_ROLE frameworks.

/authorizer.cpp (line 247) <https://reviews.apache.org/r/56178/#comment236726> MESOS-7091? - Adam B On Feb. 9, 2017, 12:59 a.m., Benjamin Bannier wrote: > > --- > This is an automatically generated e-mail. To reply,

Re: Review Request 56178: Enabled the authorizer to work with MULTI_ROLE frameworks.

> On Feb. 3, 2017, 2:26 a.m., Adam B wrote: > > src/authorizer/local/authorizer.cpp, line 244 > > <https://reviews.apache.org/r/56178/diff/1/?file=1621348#file1621348line244> > > > > Seems like framework_info is always set, so how/why would we ever fal

Re: Review Request 56178: Enabled the authorizer to work with MULTI_ROLE frameworks.

() <= 1)` instead of checking the capability? A legacy authorizer could still authorize a multi-role-capable framework if it's only trying to register with a single role. - Adam B On Feb. 7, 2017, 2:26 a.m., B

Re: Review Request 56178: Enabled the authorizer to work with MULTI_ROLE frameworks.

> On Feb. 3, 2017, 2:26 a.m., Adam B wrote: > > src/authorizer/local/authorizer.cpp, line 244 > > <https://reviews.apache.org/r/56178/diff/1/?file=1621348#file1621348line244> > > > > Seems like framework_info is always set, so how/why would we ever fal

Re: Review Request 56178: Enabled the authorizer to work with MULTI_ROLE frameworks.

> On Feb. 7, 2017, 1:16 a.m., Adam B wrote: > > src/master/master.cpp, line 2178 > > <https://reviews.apache.org/r/56178/diff/5/?file=1626265#file1626265line2178> > > > > What does the framework's capability have to do with whether a custom >

Re: Review Request 56178: Enabled the authorizer to work with MULTI_ROLE frameworks.

> On Feb. 7, 2017, 1:16 a.m., Adam B wrote: > > src/master/master.cpp, line 2178 > > <https://reviews.apache.org/r/56178/diff/5/?file=1626265#file1626265line2178> > > > > What does the framework's capability have to do with whether a custom >

Re: Review Request 56359: Add framework principal to the slave state endpoint

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56359/#review164822 --- Ship it! Ship It! - Adam B On Feb. 6, 2017, 6:14 p.m., Jeff

Re: Review Request 56178: Enabled the authorizer to work with MULTI_ROLE frameworks.

lly provides a back-door for those frameworks to use other roles. Or I could call the authorizer multiple times, one for each role. But we wouldn't want to do that for multi-role capable authorizers. Maybe we need the concept of "module capabilites" now too? Am I overthinking this?

Re: Review Request 56178: Enabled the authorizer to work with MULTI_ROLE frameworks.

> On Feb. 3, 2017, 2:26 a.m., Adam B wrote: > > src/authorizer/local/authorizer.cpp, line 244 > > <https://reviews.apache.org/r/56178/diff/1/?file=1621348#file1621348line244> > > > > Seems like framework_info is always set, so how/why would we ever fal

Re: Review Request 56178: Enabled the authorizer to work with MULTI_ROLE frameworks.

t; s/use/used/ - Adam B On Feb. 1, 2017, 9:06 a.m., Benjamin Bannier wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56178/ >

Re: Review Request 56052: Added the 'Secret' protobuf message.

tps://reviews.apache.org/r/56052/#comment235683> What are these two fields used for? Can you give an example to demonstrate the difference between name and key? - Adam B On Feb. 2, 2017, 11:39 a.m., Greg Mann

Re: Review Request 53998: Fixed hook to allow for executor environment modifications.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53998/#review162967 --- Ping. Do we still need this? - Adam B On Nov. 22, 2016, 12:26

Re: Review Request 55152: Fixed nested container docs and added a link to home.md.

generate the website, rather than just following github markdown? - Adam B On Jan. 3, 2017, 1:44 p.m., Gilbert Song wrote: > > --- > This is an automatically generated e-mail. To reply, visit: >

Re: Review Request 54803: Fixed `SlaveTests` to pass when `HAS_AUTHENTICATION` is undefined.

che.org/r/54803/#comment231000> Why did you need to move the pause all the way up here, instead of just before the first `advance`? Please add a comment about why we're pausing. - Adam B On Dec. 20, 2016, 2:32 p.m., Alex C

Re: Review Request 54731: Fixed a check bug in LAUNCH_NESTED_CONTAINER_SESSION_CALL.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54731/#review159108 --- Ship it! Ship It! - Adam B On Dec. 13, 2016, 8:15 p.m

Re: Review Request 54662: Enabled authorization in SET_LOG_LEVEL API call.

y. src/slave/http.cpp (lines 905 - 906) <https://reviews.apache.org/r/54662/#comment230042> Looks like weird wrapping to me. I'd prefer to see it wrapped after `](` like other lambdas - Adam B On Dec. 12, 2016, 6:37 a.m., Alexan

Re: Review Request 54538: Enabled fine grained authorization for the getContainers API Call.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54538/#review159091 --- Ship it! Ship It! - Adam B On Dec. 13, 2016, 6:47 a.m

Re: Review Request 54661: Enable authorization for the GET_FLAGS API Call.

tps://reviews.apache.org/r/54661/#comment230041> Looks like excessive `()` wrapping, but I can fix this before committing. - Adam B On Dec. 13, 2016, 6:49 a.m., Alexander Rojas wrote: > > --- > This is an automatically g

Re: Review Request 54700: Fixed bug that prevented window size updates in the IOSwitchboard.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54700/#review159052 --- Ship it! Ship It! - Adam B On Dec. 13, 2016, 4:34 a.m

Re: Review Request 54538: Enabled fine grained authorization for the getContainers API Call.

. src/slave/http.cpp (line 1869) <https://reviews.apache.org/r/54538/#comment229880> Each parameter on a separate line, please src/slave/http.cpp (line 1929) <https://reviews.apache.org/r/54538/#comment229884> Already called `info` a few lines above. Can you reuse that? - Ada

  1   2   3   4   5   6   7   >