Re: Review Request 38580: Added docker registry RemotePuller

2015-11-08 Thread Jojy Varghese
> On Nov. 7, 2015, 9:36 a.m., Timothy Chen wrote: > > src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp, line 51 > > > > > > This is no longer true since we pulled these out of RemotePuller right?

Re: Review Request 38580: Added docker registry RemotePuller

2015-11-08 Thread Jojy Varghese
> On Nov. 4, 2015, 9:50 p.m., Timothy Chen wrote: > > src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp, line 51 > > > > > > Didn''t you pull these out? > > Jojy Varghese wrote: > Not sure what

Re: Review Request 40054: Fix fetch parsing problem for URL with query.

2015-11-08 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40054/#review105613 --- Bad patch! Reviews applied: [40053] Failed command:

Re: Review Request 40056: Make hook execution order deterministic.

2015-11-08 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40056/#review105612 --- Bad patch! Reviews applied: [40056] Failed command:

Re: Review Request 40054: Fix fetch parsing problem for URL with query.

2015-11-08 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40054/ --- (Updated Nov. 8, 2015, 1:34 p.m.) Review request for mesos, Bernd Mathiske and

Re: Review Request 40054: Fix fetch parsing problem for URL with query.

2015-11-08 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40054/ --- (Updated Nov. 8, 2015, 3:57 p.m.) Review request for mesos, Bernd Mathiske and

Re: Review Request 40053: Add URL::parse function.

2015-11-08 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40053/ --- (Updated Nov. 8, 2015, 3:56 p.m.) Review request for mesos, Bernd Mathiske and

Re: Review Request 39018: Added JSON parsing for Resources.

2015-11-08 Thread Adam B
> On Nov. 6, 2015, 10:13 a.m., Greg Mann wrote: > > src/tests/resources_tests.cpp, lines 593-613 > > > > > > I fixed an unintended error in this jsonString, and then found that the > > parsing didn't return an

Re: Review Request 39018: Added JSON parsing for Resources.

2015-11-08 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39018/#review105607 --- Almost there, but I won't let you "silently ignore" any errors on

Review Request 40056: Make hook execution order deterministic.

2015-11-08 Thread haosdent huang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40056/ --- Review request for mesos, Ben Mahler and Kapil Arya. Bugs: MESOS-3485

Re: Review Request 40054: Fix fetch parsing problem for URL with query.

2015-11-08 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40054/#review105618 --- Patch looks great! Reviews applied: [40053, 40054] All tests

Re: Review Request 38580: Added docker registry RemotePuller

2015-11-08 Thread Jojy Varghese
> On Nov. 7, 2015, 9:36 a.m., Timothy Chen wrote: > > src/slave/containerizer/mesos/provisioner/docker/remote_puller.cpp, line 246 > > > > > > Can there be layers that actually have zero content? Docker images > >

Re: Review Request 38218: Quota: Extended the Allocator interface with quota-related methods.

2015-11-08 Thread Alexander Rukletsov
> On Nov. 6, 2015, 2:40 p.m., Joerg Schad wrote: > > include/mesos/master/allocator.hpp, line 357 > > > > > > s/given role not set/nonexisting role? This sentence refers to an existing role without quota. Do you

Re: Review Request 38218: Quota: Extended the Allocator interface with quota-related methods.

2015-11-08 Thread Alexander Rukletsov
> On Nov. 2, 2015, 2:22 p.m., Joerg Schad wrote: > > src/master/allocator/mesos/allocator.hpp, line 268 > > > > > > Could we add a todo for updateQuota()? In my opinion this would make it > > easier to understand

Re: Review Request 38218: Quota: Extended the Allocator interface with quota-related methods.

2015-11-08 Thread Alexander Rukletsov
> On Nov. 4, 2015, 11:24 p.m., Joris Van Remoortere wrote: > > include/mesos/master/allocator.hpp, lines 332-347 > > > > > > do you want to re-order these (here or in the implementations) so that > > the function

Re: Review Request 38218: Quota: Extended the Allocator interface with quota-related methods.

2015-11-08 Thread Alexander Rukletsov
> On Nov. 5, 2015, 7:09 p.m., Joseph Wu wrote: > > include/mesos/master/allocator.hpp, lines 355-356 > > > > > > This bit is somewhat unclear: > > "the group of roles with quota set" sounds like it is referring

Re: Review Request 38218: Quota: Extended the Allocator interface with quota-related methods.

2015-11-08 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38218/ --- (Updated Nov. 8, 2015, 11:06 p.m.) Review request for mesos, Bernd Mathiske,

Re: Review Request 39285: Added Quota Request Validation.

2015-11-08 Thread Alexander Rukletsov
> On Nov. 6, 2015, 4:19 p.m., Klaus Ma wrote: > > src/master/quota_handler.cpp, line 119 > > > > > > Should we also check whether `resource.get().role()` is empty? There > > should be the case that assign empty

Review Request 40062: MESIS-2315 Deprecate/Remove CommandInfo::ContainerInfo

2015-11-08 Thread Vaibhav Khanduja
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40062/ --- Review request for mesos and Adam B. Repository: mesos Description ---

Re: Review Request 40062: MESIS-2315 Deprecate/Remove CommandInfo::ContainerInfo

2015-11-08 Thread Vaibhav Khanduja
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40062/ --- (Updated Nov. 9, 2015, 1:06 a.m.) Review request for mesos and Adam B.

Re: Review Request 40062: MESOS-2315 Deprecate/Remove CommandInfo::ContainerInfo

2015-11-08 Thread Vaibhav Khanduja
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40062/ --- (Updated Nov. 9, 2015, 1:16 a.m.) Review request for mesos and Adam B.

Re: Review Request 39317: Quota: Moved `QuotaInfo` protobuf into a separate package.

2015-11-08 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39317/ --- (Updated Nov. 8, 2015, 11 p.m.) Review request for mesos, Bernd Mathiske,

Re: Review Request 39285: Added Quota Request Validation.

2015-11-08 Thread Alexander Rukletsov
> On Nov. 6, 2015, 4:19 p.m., Klaus Ma wrote: > > src/master/quota_handler.cpp, line 180 > > > > > > Should we move it into `validateQuotaRequest`? If any role is exist in > > master, we did not need to continue to

Re: Review Request 39223: Added Quota Request Validation Tests.

2015-11-08 Thread Guangya Liu
> On Nov. 8, 2015, 2:24 a.m., Guangya Liu wrote: > > src/tests/master_quota_tests.cpp, lines 209-210 > > > > > > Can you please make this comments limit in 70 per line and ditto for > > the following? > >

Re: Review Request 40062: MESIS-2315 Deprecate/Remove CommandInfo::ContainerInfo

2015-11-08 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40062/#review105631 --- Bad patch! Reviews applied: [40062] Failed command:

Re: Review Request 38218: Quota: Extended the Allocator interface with quota-related methods.

2015-11-08 Thread Guangya Liu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38218/#review105638 --- Ship it! Ship It! - Guangya Liu On Nov. 8, 2015, 11:06 p.m.,

Re: Review Request 39459: Added docs for using delay() and clocks in libprocess.

2015-11-08 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39459/#review105642 --- Ship it! Thanks Neil! I made a few minor edits so folks that copy

Re: Review Request 39453: Added HTTP docs to libprocess README.md.

2015-11-08 Thread Benjamin Hindman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39453/#review105641 --- Ship it! Made a few minor edits to make this code compilable

Re: Review Request 39317: Quota: Moved QuotaInfo protobuf into a separate package.

2015-11-08 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39317/ --- (Updated Nov. 8, 2015, 10:53 p.m.) Review request for mesos, Bernd Mathiske,

Re: Review Request 39223: Added Quota Request Validation Tests.

2015-11-08 Thread Alexander Rukletsov
> On Nov. 8, 2015, 2:24 a.m., Guangya Liu wrote: > > src/tests/master_quota_tests.cpp, lines 209-210 > > > > > > Can you please make this comments limit in 70 per line and ditto for > > the following? Why do you

Re: Review Request 40062: MESOS-2315 Deprecate/Remove CommandInfo::ContainerInfo

2015-11-08 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40062/#review105636 --- Patch looks great! Reviews applied: [40062] All tests passed. -

Re: Review Request 38218: Quota: Extended the Allocator interface with quota-related methods.

2015-11-08 Thread Guangya Liu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38218/#review105640 --- Ship it! Ship It! - Guangya Liu On Nov. 8, 2015, 11:06 p.m.,

Re: Review Request 39018: Added JSON parsing for Resources.

2015-11-08 Thread Greg Mann
> On Nov. 8, 2015, 9:32 a.m., Adam B wrote: > > src/common/resources.cpp, line 271 > > > > > > Red flag: "silently ignored" scares me. Don't you mean that an error in > > validate() will yield an empty

Re: Review Request 39989: [5/5] Added framework authorization for dynamic reservation.

2015-11-08 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39989/ --- (Updated Nov. 9, 2015, 6:13 a.m.) Review request for mesos, Adam B, Jie Yu,

Re: Review Request 40070: Refactored registry puller pull to smaller functions.

2015-11-08 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40070/#review105656 --- Bad patch! Reviews applied: [39832, 39882, 39839, 38579, 39015,

Re: Review Request 39250: Puller refactor: moved untar to a common place

2015-11-08 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39250/ --- (Updated Nov. 9, 2015, 7:23 a.m.) Review request for mesos and Timothy Chen.

Re: Review Request 39250: Puller refactor: moved untar to a common place

2015-11-08 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39250/#review105658 --- Bad patch! Reviews applied: [38747] Failed command:

Re: Review Request 39102: Added documentation for JSON resources.

2015-11-08 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39102/#review105611 --- Ship it! Ship It! - Adam B On Nov. 6, 2015, 4:18 p.m., Greg

Re: Review Request 40054: Fix fetch parsing problem for URL with query.

2015-11-08 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40054/#review105615 --- Bad patch! Reviews applied: [40053] Failed command:

Re: Review Request 39018: Added JSON parsing for Resources.

2015-11-08 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39018/ --- (Updated Nov. 9, 2015, 5:57 a.m.) Review request for mesos, Adam B, Alexander

Re: Review Request 39987: [3/5] Added 'Master::authorize' for Reserve/Unreserve.

2015-11-08 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39987/ --- (Updated Nov. 9, 2015, 6:10 a.m.) Review request for mesos, Adam B, Jie Yu,

Re: Review Request 39986: [2/5] Enabled the Authorizer to handle Reserve/Unreserve ACLs.

2015-11-08 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39986/ --- (Updated Nov. 9, 2015, 6:09 a.m.) Review request for mesos, Adam B, Jie Yu,

Review Request 40070: Refactored registry puller pull to smaller functions.

2015-11-08 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40070/ --- Review request for mesos and Jojy Varghese. Repository: mesos Description

Re: Review Request 37168: MESOS-3063 (Add an example framework using dynamic reservation)

2015-11-08 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37168/#review105651 --- Patch looks great! Reviews applied: [37168] All tests passed. -

Re: Review Request 39018: Added JSON parsing for Resources.

2015-11-08 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39018/#review105653 --- Ship it! Much better error handling, but we're still dropping an

Re: Review Request 37168: MESOS-3063 (Add an example framework using dynamic reservation)

2015-11-08 Thread Klaus Ma
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/37168/ --- (Updated Nov. 9, 2015, 1:29 p.m.) Review request for mesos and Michael Park.

Review Request 40069: Renamed RemotePuller to RegistryPuller.

2015-11-08 Thread Timothy Chen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40069/ --- Review request for mesos and Jojy Varghese. Repository: mesos Description

Re: Review Request 40069: Renamed RemotePuller to RegistryPuller.

2015-11-08 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40069/#review105652 --- Ship it! Ship It! - Jojy Varghese On Nov. 9, 2015, 6:21 a.m.,

Re: Review Request 40070: Refactored registry puller pull to smaller functions.

2015-11-08 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40070/#review105657 --- Ship it! Ship It!

Re: Review Request 39223: Added Quota Request Validation Tests.

2015-11-08 Thread Joerg Schad
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39223/ --- (Updated Nov. 9, 2015, 7:29 a.m.) Review request for mesos, Alexander

Re: Review Request 39285: Added Quota Request Validation.

2015-11-08 Thread Joerg Schad
> On Nov. 6, 2015, 4:19 p.m., Klaus Ma wrote: > > src/master/quota_handler.cpp, line 180 > > > > > > Should we move it into `validateQuotaRequest`? If any role is exist in > > master, we did not need to continue to

Re: Review Request 39250: Puller refactor: moved untar to a common place

2015-11-08 Thread Jojy Varghese
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39250/ --- (Updated Nov. 9, 2015, 7:55 a.m.) Review request for mesos and Timothy Chen.