Re: Review Request 52642: Improved the validation of RESERVE operations.

2016-11-08 Thread Gastón Kleiman
> On Nov. 8, 2016, 10:37 a.m., Alexander Rukletsov wrote: > > src/tests/master_validation_tests.cpp, lines 311-313 > > > > > > This test does not do what it says. It's a copy-paste of > >

Re: Review Request 52642: Improved the validation of RESERVE operations.

2016-11-08 Thread Gastón Kleiman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52642/ --- (Updated Nov. 8, 2016, 10:45 a.m.) Review request for mesos, Alexander

Re: Review Request 52642: Improved the validation of RESERVE operations.

2016-11-08 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52642/#review155276 --- Fix it, then Ship it! I'll fix the formatting issues and will

Re: Review Request 52642: Improved the validation of RESERVE operations.

2016-11-08 Thread Gastón Kleiman
> On Nov. 3, 2016, 10:39 p.m., Benjamin Mahler wrote: > > src/tests/master_validation_tests.cpp, lines 346-348 > > > > > > Have you gone through these tests? They also seem confusing. For > > example, this one says

Re: Review Request 52642: Improved the validation of RESERVE operations.

2016-11-07 Thread Gastón Kleiman
> On Nov. 3, 2016, 10:39 p.m., Benjamin Mahler wrote: > > src/tests/master_validation_tests.cpp, line 343 > > > > > > Shouldn't we be checking that a framework can't remove a reservation > > for a role that doesn't

Re: Review Request 52642: Improved the validation of RESERVE operations.

2016-11-07 Thread Gastón Kleiman
> On Nov. 3, 2016, 10:39 p.m., Benjamin Mahler wrote: > > src/tests/master_validation_tests.cpp, lines 207-218 > > > > > > Hm.. it's too bad this test wasn't doing what its comment said, > > otherwise we'd have

Re: Review Request 52642: Improved the validation of RESERVE operations.

2016-11-07 Thread Gastón Kleiman
> On Nov. 3, 2016, 10:39 p.m., Benjamin Mahler wrote: > > src/tests/master_validation_tests.cpp, line 343 > > > > > > Shouldn't we be checking that a framework can't remove a reservation > > for a role that doesn't

Re: Review Request 52642: Improved the validation of RESERVE operations.

2016-11-07 Thread Gastón Kleiman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52642/ --- (Updated Nov. 7, 2016, 11:02 a.m.) Review request for mesos, Alexander

Re: Review Request 52642: Improved the validation of RESERVE operations.

2016-11-04 Thread Gastón Kleiman
> On Nov. 3, 2016, 10:39 p.m., Benjamin Mahler wrote: > > src/tests/master_validation_tests.cpp, lines 346-348 > > > > > > Have you gone through these tests? They also seem confusing. For > > example, this one says

Re: Review Request 52642: Improved the validation of RESERVE operations.

2016-11-04 Thread Gastón Kleiman
> On Nov. 4, 2016, 11:04 a.m., Alexander Rukletsov wrote: > > src/master/validation.hpp, line 188 > > > > > > Will `allowedRole` or `frameworkRole` be more descriptive? I went for `frameworkRole`. - Gastón

Re: Review Request 52642: Improved the validation of RESERVE operations.

2016-11-04 Thread Gastón Kleiman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52642/ --- (Updated Nov. 4, 2016, 12:50 p.m.) Review request for mesos, Alexander

Re: Review Request 52642: Improved the validation of RESERVE operations.

2016-11-04 Thread Gastón Kleiman
> On Nov. 4, 2016, 11:04 a.m., Alexander Rukletsov wrote: > > src/master/master.cpp, line 3855 > > > > > > `FrameworkInfo.role` is optional, though with a default value. > > > > First, do we want to rely

Re: Review Request 52642: Improved the validation of RESERVE operations.

2016-11-04 Thread Gastón Kleiman
> On Nov. 3, 2016, 10:39 p.m., Benjamin Mahler wrote: > > src/master/validation.cpp, lines 1454-1456 > > > > > > Looks like you forgot to close the quote on the last line. Can you put > > quotes on the same line?

Re: Review Request 52642: Improved the validation of RESERVE operations.

2016-11-04 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52642/#review154888 --- Let's split the fix and the test into separate patches.

Re: Review Request 52642: Improved the validation of RESERVE operations.

2016-11-03 Thread Benjamin Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52642/#review154824 --- Thanks Gaston! The fix looks good, but some of the tests don't

Re: Review Request 52642: Improved the validation of RESERVE operations.

2016-10-24 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52642/#review153766 --- Patch looks great! Reviews applied: [52642] Passed command:

Re: Review Request 52642: Improved the validation of RESERVE operations.

2016-10-24 Thread Gastón Kleiman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52642/ --- (Updated Oct. 24, 2016, 3:25 p.m.) Review request for mesos, Alexander

Re: Review Request 52642: Improved the validation of RESERVE operations.

2016-10-07 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52642/#review151906 --- Patch looks great! Reviews applied: [52642] Passed command:

Review Request 52642: Improved the validation of RESERVE operations.

2016-10-07 Thread Gastón Kleiman
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52642/ --- Review request for mesos and Alexander Rukletsov. Bugs: MESOS-6142