Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/#review106390 --- Ship it! Ship It! - Joris Van Remoortere On Nov. 12, 2015,

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/#review106402 --- src/master/quota_handler.cpp (lines 95 - 102)

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/#review106426 --- src/master/quota_handler.cpp (lines 107 - 111)

Re: Review Request 39285: Added Quota Request Validation.

> On Nov. 13, 2015, 12:03 p.m., Alexander Rukletsov wrote: > > src/master/quota_handler.cpp, lines 95-102 > > > > > > Not your fault, but here is one thing that bothers me a bit. > > > > It looks like we

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/#review106405 --- src/master/master.hpp (line 878)

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/ --- (Updated Nov. 12, 2015, 10:26 a.m.) Review request for mesos, Alexander

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/#review106221 --- src/master/quota_handler.cpp (line 114)

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/ --- (Updated Nov. 12, 2015, 3:21 p.m.) Review request for mesos, Alexander

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/ --- (Updated Nov. 11, 2015, 5:35 p.m.) Review request for mesos, Alexander

Re: Review Request 39285: Added Quota Request Validation.

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

Re: Review Request 39285: Added Quota Request Validation.

> On Nov. 11, 2015, 6:48 p.m., Alexander Rukletsov wrote: > > One thing I think is not entirely clean is repetition of some validation > > checks. For example, you check whether a role is set twice: while > > constructing a `QuotaInfo` instance and while validating it. I think we can > >

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/#review106118 --- I have troubles seeing the whole patch. Maybe it's because a

Re: Review Request 39285: Added Quota Request Validation.

> On Nov. 11, 2015, 6:48 p.m., Alexander Rukletsov wrote: > > src/master/quota_handler.cpp, line 148 > > > > > > You call it "request query string" above, any reason you change the > > name? > > Joerg Schad

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/#review106122 --- One thing I think is not entirely clean is repetition of some

Re: Review Request 39285: Added Quota Request Validation.

joerg, can you please make sure these reviews have linear dependencies? i'm seeing some reviews that block multiple reviews!? are you not using the post-reviews script? On Tue, Nov 10, 2015 at 10:20 AM, Joerg Schad wrote: > >

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/#review105913 --- src/master/quota.cpp (line 36)

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/ --- (Updated Nov. 10, 2015, 6:10 p.m.) Review request for mesos, Alexander

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/ --- (Updated Nov. 10, 2015, 6:20 p.m.) Review request for mesos, Alexander

Re: Review Request 39285: Added Quota Request Validation.

The tree structure of the reviews is intentional (at the moment). It's currently roughly split into components like: / Registrar / POST Protobufs - /quota endpoint - Authentication \ Allocator \ \ GET \- DELETE ~Joseph On Tue, Nov 10,

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/ --- (Updated Nov. 10, 2015, 8:37 a.m.) Review request for mesos, Alexander

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/#review105772 --- src/master/quota_handler.cpp (line 75)

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/ --- (Updated Nov. 10, 2015, 9:07 a.m.) Review request for mesos, Alexander

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/ --- (Updated Nov. 10, 2015, 10 a.m.) Review request for mesos, Alexander

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/ --- (Updated Nov. 10, 2015, 2:03 p.m.) Review request for mesos, Alexander

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/#review105803 --- src/master/master.hpp (line 876)

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/#review105668 --- Ship it! Ship It! - Klaus Ma On Nov. 9, 2015, 8:51 p.m., Joerg

Re: Review Request 39285: Added Quota Request Validation.

> On Nov. 7, 2015, 12:19 a.m., Klaus Ma wrote: > > src/master/quota_handler.cpp, line 140 > > > > > > Suggest to move it into the loop; if any role is not known by master, > > we did not need to continue to check

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/ --- (Updated Nov. 9, 2015, 12:51 p.m.) Review request for mesos, Alexander

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/ --- (Updated Nov. 9, 2015, 7:31 p.m.) Review request for mesos, Alexander

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/ --- (Updated Nov. 9, 2015, 8:25 p.m.) Review request for mesos, Alexander

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/ --- (Updated Nov. 9, 2015, 11:30 p.m.) Review request for mesos, Alexander

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/ --- (Updated Nov. 9, 2015, 8:16 a.m.) Review request for mesos, Alexander

Re: Review Request 39285: Added Quota Request Validation.

> 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

Re: Review Request 39285: Added Quota Request Validation.

> 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 39285: Added Quota Request Validation.

> 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 39285: Added Quota Request Validation.

> On Nov. 5, 2015, 9:59 p.m., Joseph Wu wrote: > > src/master/quota_handler.cpp, lines 102-110 > > > > > > What do you think about using helpers from > > `include/mesos/resources.hpp`? A bunch of these checks

Re: Review Request 39285: Added Quota Request Validation.

> On Nov. 3, 2015, 1:37 p.m., Qian Zhang wrote: > > src/master/quota_handler.cpp, line 123 > > > > > > Suggest to change ```if``` to ```else if```, in this way, for the first > > resource, we do not need to compare

Re: Review Request 39285: Added Quota Request Validation.

> On Oct. 25, 2015, 2:45 a.m., Klaus Ma wrote: > > src/master/quota_handler.cpp, line 115 > > > > > > It said the role maybe not set in allocator interface's comments > > (`setQuota`); so any case that the role

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/ --- (Updated Nov. 6, 2015, 3:23 p.m.) Review request for mesos, Alexander

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/#review105440 --- src/master/quota_handler.cpp (line 113)

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/#review105450 --- src/master/quota_handler.cpp (line 118)

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/#review105313 --- src/master/quota_handler.cpp (lines 93 - 101)

Re: Review Request 39285: Added Quota Request Validation.

> On Nov. 2, 2015, 8:43 a.m., Alexander Rukletsov wrote: > > src/master/quota_handler.cpp, lines 148-149 > > > > > > As per @joris' comment in the previous review, should we check for > > errors after `.get()`

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/#review104906 --- src/master/quota_handler.cpp (line 113)

Re: Review Request 39285: Added Quota Request Validation.

> On Oct. 25, 2015, 2:45 a.m., Klaus Ma wrote: > > src/master/master.hpp, line 864 > > > > > > Should we return Bad Request (404) for now, because we did not > > implement it yet. As it is implemented in the

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/#review104725 --- Ship it! Looks good! The only thing I'm not sure about is how to

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/#review104861 --- src/master/quota_handler.cpp (lines 157 - 158)

Re: Review Request 39285: Added Quota Request Validation.

> On 十一月 2, 2015, 4:43 p.m., Alexander Rukletsov wrote: > > src/master/quota_handler.cpp, line 84 > > > > > > Mind adding a comment this is a "reference" role which is deduced from > > the resources in the request

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/ --- (Updated Oct. 24, 2015, 7:42 p.m.) Review request for mesos, Alexander

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/#review103918 --- src/master/master.hpp (line 864)

Re: Review Request 39285: Added Quota Request Validation.

> On Oct. 22, 2015, 10:25 a.m., Alexander Rukletsov wrote: > > src/master/quota_handler.cpp, lines 71-72 > > > > > > How about initializing `role` with QuotaInfo.role? Or we agreed not to > > send role as part of

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/ --- (Updated Oct. 23, 2015, 4:52 p.m.) Review request for mesos, Alexander

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/#review103613 --- src/master/master.hpp (lines 883 - 884)

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/#review103539 --- src/master/master.hpp (line 878)

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/#review103587 --- src/master/quota_handler.cpp (line 162)

Re: Review Request 39285: Added Quota Request Validation.

> On Oct. 22, 2015, 10:57 a.m., Alexander Rukletsov wrote: > > src/master/quota_handler.cpp, lines 64-66 > > > > > > This is an API change which contradicts the API described in the design > > doc and differs from,

Re: Review Request 39285: Added Quota Request Validation.

> On Oct. 22, 2015, 10:25 a.m., Alexander Rukletsov wrote: > > src/master/master.hpp, line 878 > > > > > > s/Check/Checks > > s/Request/request > > > > Do we capitalize "quota"? I did, will adjust to

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/ --- (Updated Oct. 22, 2015, 4:38 a.m.) Review request for mesos, Alexander

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/#review103360 --- src/master/quota_handler.cpp (lines 29 - 31)

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/ --- (Updated Oct. 19, 2015, 8:11 p.m.) Review request for mesos, Alexander

Re: Review Request 39285: Added Quota Request Validation.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39285/ --- (Updated Oct. 15, 2015, 6:50 p.m.) Review request for mesos, Alexander