You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Bruce Merry <bm...@ska.ac.za> on 2017/01/20 13:01:14 UTC
Review Request 55761: Fixed name matching for automatic resources.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55761/
-----------------------------------------------------------
Review request for mesos and Vinod Kone.
Bugs: MESOS-6821
https://issues.apache.org/jira/browse/MESOS-6821
Repository: mesos
Description
-------
Instead of doing a simple substring search for known resources like
"cpus" to decide whether they have been explicitly specified, it now
parses the resource string to check that a resource with that exact
name was specified.
This splits out a fromString static member function from the start of
Resources::parse that does parsing using either fromJSON or
fromSimpleString and makes it available to use in
mesos::internal::slave::Containerizer::resources.
Fixes MESOS-6821.
Diffs
-----
include/mesos/resources.hpp c2e6d6680d9687f0bbfb73e84ffb1457072c4412
src/Makefile.am 3d0e82cb522d379923e00e5e98e9c9f646fdd3db
src/common/resources.cpp be9bca2063e9f0e60c5faa0142077bea56272e45
src/slave/containerizer/containerizer.cpp d46882baa904fd439bffb23c324828b777228f1c
src/tests/containerizer/containerizer_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/55761/diff/
Testing
-------
Regression test added.
Thanks,
Bruce Merry
Re: Review Request 55761: Fixed name matching for automatic resources.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55761/#review162430
-----------------------------------------------------------
Patch looks great!
Reviews applied: [55761]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On Jan. 20, 2017, 1:01 p.m., Bruce Merry wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55761/
> -----------------------------------------------------------
>
> (Updated Jan. 20, 2017, 1:01 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-6821
> https://issues.apache.org/jira/browse/MESOS-6821
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Instead of doing a simple substring search for known resources like
> "cpus" to decide whether they have been explicitly specified, it now
> parses the resource string to check that a resource with that exact
> name was specified.
>
> This splits out a fromString static member function from the start of
> Resources::parse that does parsing using either fromJSON or
> fromSimpleString and makes it available to use in
> mesos::internal::slave::Containerizer::resources.
>
> Fixes MESOS-6821.
>
>
> Diffs
> -----
>
> include/mesos/resources.hpp c2e6d6680d9687f0bbfb73e84ffb1457072c4412
> src/Makefile.am 3d0e82cb522d379923e00e5e98e9c9f646fdd3db
> src/common/resources.cpp be9bca2063e9f0e60c5faa0142077bea56272e45
> src/slave/containerizer/containerizer.cpp d46882baa904fd439bffb23c324828b777228f1c
> src/tests/containerizer/containerizer_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/55761/diff/
>
>
> Testing
> -------
>
> Regression test added.
>
>
> Thanks,
>
> Bruce Merry
>
>
Re: Review Request 55761: Fixed name matching for automatic resources.
Posted by Bruce Merry <bm...@ska.ac.za>.
> On Jan. 24, 2017, 1:47 a.m., Vinod Kone wrote:
> > src/tests/containerizer/containerizer_tests.cpp, lines 76-77
> > <https://reviews.apache.org/r/55761/diff/1/?file=1610537#file1610537line76>
> >
> > also, can you just use "stringify" here?
>
> Bruce Merry wrote:
> I don't follow. What do you want me to stringify? (I'm not familiar with which Mesos classes can be stringified and what the string representation yields).
>
> Vinod Kone wrote:
> So we have `stringify` method defined here https://github.com/apache/mesos/blob/master/3rdparty/stout/include/stout/stringify.hpp. It is templated, so it will work for any class as long as it has an output stream operator method defined.
>
> `Resources::ports()` returns Option<Value::Ranges> and `Value::Ranges` has an ostream operator defined here https://github.com/apache/mesos/blob/master/src/common/values.cpp#L308
>
> Let me know if this doesn't work for some reason.
Gotcha, I'd missed the idea of changing `resources->get("ports")` to `resources->ports()`. It works.
- Bruce
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55761/#review162753
-----------------------------------------------------------
On Jan. 25, 2017, 7:10 a.m., Bruce Merry wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55761/
> -----------------------------------------------------------
>
> (Updated Jan. 25, 2017, 7:10 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-6821
> https://issues.apache.org/jira/browse/MESOS-6821
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Instead of doing a simple substring search for known resources like
> "cpus" to decide whether they have been explicitly specified, it now
> parses the resource string to check that a resource with that exact
> name was specified.
>
> This splits out a fromString static member function from the start of
> Resources::parse that does parsing using either fromJSON or
> fromSimpleString and makes it available to use in
> mesos::internal::slave::Containerizer::resources.
>
> Fixes MESOS-6821.
>
>
> Diffs
> -----
>
> include/mesos/resources.hpp c2e6d6680d9687f0bbfb73e84ffb1457072c4412
> src/Makefile.am 3d0e82cb522d379923e00e5e98e9c9f646fdd3db
> src/common/resources.cpp be9bca2063e9f0e60c5faa0142077bea56272e45
> src/slave/containerizer/containerizer.cpp d46882baa904fd439bffb23c324828b777228f1c
> src/tests/containerizer/containerizer_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/55761/diff/
>
>
> Testing
> -------
>
> Regression test added.
>
>
> Thanks,
>
> Bruce Merry
>
>
Re: Review Request 55761: Fixed name matching for automatic resources.
Posted by Vinod Kone <vi...@gmail.com>.
> On Jan. 24, 2017, 1:47 a.m., Vinod Kone wrote:
> > src/slave/containerizer/containerizer.cpp, line 82
> > <https://reviews.apache.org/r/55761/diff/1/?file=1610536#file1610536line82>
> >
> > s/parts/resources_/
>
> Bruce Merry wrote:
> Are you sure you want this? According to the style guide, "Some trailing underscores are used to distinguish between similar variables in the same scope (think prime symbols), but this should be avoided as much as possible, including removing existing instances in the code base."
>
> I agree "parts" is not the most descriptive. Maybe "resourceList"? I've changed it to that for now.
`resourceList` sgtm. Dropped the issue.
> On Jan. 24, 2017, 1:47 a.m., Vinod Kone wrote:
> > src/tests/containerizer/containerizer_tests.cpp, lines 76-77
> > <https://reviews.apache.org/r/55761/diff/1/?file=1610537#file1610537line76>
> >
> > also, can you just use "stringify" here?
>
> Bruce Merry wrote:
> I don't follow. What do you want me to stringify? (I'm not familiar with which Mesos classes can be stringified and what the string representation yields).
So we have `stringify` method defined here https://github.com/apache/mesos/blob/master/3rdparty/stout/include/stout/stringify.hpp. It is templated, so it will work for any class as long as it has an output stream operator method defined.
`Resources::ports()` returns Option<Value::Ranges> and `Value::Ranges` has an ostream operator defined here https://github.com/apache/mesos/blob/master/src/common/values.cpp#L308
Let me know if this doesn't work for some reason.
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55761/#review162753
-----------------------------------------------------------
On Jan. 24, 2017, 2:59 p.m., Bruce Merry wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55761/
> -----------------------------------------------------------
>
> (Updated Jan. 24, 2017, 2:59 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-6821
> https://issues.apache.org/jira/browse/MESOS-6821
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Instead of doing a simple substring search for known resources like
> "cpus" to decide whether they have been explicitly specified, it now
> parses the resource string to check that a resource with that exact
> name was specified.
>
> This splits out a fromString static member function from the start of
> Resources::parse that does parsing using either fromJSON or
> fromSimpleString and makes it available to use in
> mesos::internal::slave::Containerizer::resources.
>
> Fixes MESOS-6821.
>
>
> Diffs
> -----
>
> include/mesos/resources.hpp c2e6d6680d9687f0bbfb73e84ffb1457072c4412
> src/Makefile.am 3d0e82cb522d379923e00e5e98e9c9f646fdd3db
> src/common/resources.cpp be9bca2063e9f0e60c5faa0142077bea56272e45
> src/slave/containerizer/containerizer.cpp d46882baa904fd439bffb23c324828b777228f1c
> src/tests/containerizer/containerizer_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/55761/diff/
>
>
> Testing
> -------
>
> Regression test added.
>
>
> Thanks,
>
> Bruce Merry
>
>
Re: Review Request 55761: Fixed name matching for automatic resources.
Posted by Bruce Merry <bm...@ska.ac.za>.
> On Jan. 24, 2017, 1:47 a.m., Vinod Kone wrote:
> > src/slave/containerizer/containerizer.cpp, line 82
> > <https://reviews.apache.org/r/55761/diff/1/?file=1610536#file1610536line82>
> >
> > s/parts/resources_/
Are you sure you want this? According to the style guide, "Some trailing underscores are used to distinguish between similar variables in the same scope (think prime symbols), but this should be avoided as much as possible, including removing existing instances in the code base."
I agree "parts" is not the most descriptive. Maybe "resourceList"? I've changed it to that for now.
> On Jan. 24, 2017, 1:47 a.m., Vinod Kone wrote:
> > src/tests/containerizer/containerizer_tests.cpp, lines 76-77
> > <https://reviews.apache.org/r/55761/diff/1/?file=1610537#file1610537line76>
> >
> > also, can you just use "stringify" here?
I don't follow. What do you want me to stringify? (I'm not familiar with which Mesos classes can be stringified and what the string representation yields).
> On Jan. 24, 2017, 1:47 a.m., Vinod Kone wrote:
> > src/slave/containerizer/containerizer.cpp, lines 73-74
> > <https://reviews.apache.org/r/55761/diff/1/?file=1610536#file1610536line73>
> >
> > What is "this list" referring to? I thought the previous comment was clear enough?
It refers to the list defined immediately below, but I've edited the comment to make it a bit closer to the original.
> On Jan. 24, 2017, 1:47 a.m., Vinod Kone wrote:
> > src/tests/containerizer/containerizer_tests.cpp, line 56
> > <https://reviews.apache.org/r/55761/diff/1/?file=1610537#file1610537line56>
> >
> > we typicall use camel case in mesos. here you can just do,
> >
> > s/expected_ports/ports/
Thanks, I'd been trying to follow the style guide but habits slipped through here.
- Bruce
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55761/#review162753
-----------------------------------------------------------
On Jan. 24, 2017, 2:59 p.m., Bruce Merry wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55761/
> -----------------------------------------------------------
>
> (Updated Jan. 24, 2017, 2:59 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-6821
> https://issues.apache.org/jira/browse/MESOS-6821
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Instead of doing a simple substring search for known resources like
> "cpus" to decide whether they have been explicitly specified, it now
> parses the resource string to check that a resource with that exact
> name was specified.
>
> This splits out a fromString static member function from the start of
> Resources::parse that does parsing using either fromJSON or
> fromSimpleString and makes it available to use in
> mesos::internal::slave::Containerizer::resources.
>
> Fixes MESOS-6821.
>
>
> Diffs
> -----
>
> include/mesos/resources.hpp c2e6d6680d9687f0bbfb73e84ffb1457072c4412
> src/Makefile.am 3d0e82cb522d379923e00e5e98e9c9f646fdd3db
> src/common/resources.cpp be9bca2063e9f0e60c5faa0142077bea56272e45
> src/slave/containerizer/containerizer.cpp d46882baa904fd439bffb23c324828b777228f1c
> src/tests/containerizer/containerizer_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/55761/diff/
>
>
> Testing
> -------
>
> Regression test added.
>
>
> Thanks,
>
> Bruce Merry
>
>
Re: Review Request 55761: Fixed name matching for automatic resources.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55761/#review162753
-----------------------------------------------------------
src/slave/containerizer/containerizer.cpp (lines 73 - 74)
<https://reviews.apache.org/r/55761/#comment234061>
What is "this list" referring to? I thought the previous comment was clear enough?
src/slave/containerizer/containerizer.cpp (line 81)
<https://reviews.apache.org/r/55761/#comment234060>
How about
// `Resources::fromString().get()` is safe because `Resources::parse()`
// above is valid.
src/slave/containerizer/containerizer.cpp (line 82)
<https://reviews.apache.org/r/55761/#comment234062>
s/parts/resources_/
src/slave/containerizer/containerizer.cpp (lines 85 - 88)
<https://reviews.apache.org/r/55761/#comment234065>
s/have/has/ ? since `Resources` is an object.
src/tests/containerizer/containerizer_tests.cpp (line 20)
<https://reviews.apache.org/r/55761/#comment234068>
new line between the two.
src/tests/containerizer/containerizer_tests.cpp (line 34)
<https://reviews.apache.org/r/55761/#comment234067>
s/ContainerizerCommonTest/ContainerizerResourcesTest/
src/tests/containerizer/containerizer_tests.cpp (line 56)
<https://reviews.apache.org/r/55761/#comment234074>
we typicall use camel case in mesos. here you can just do,
s/expected_ports/ports/
src/tests/containerizer/containerizer_tests.cpp (lines 56 - 57)
<https://reviews.apache.org/r/55761/#comment234086>
why not do this in one line as an assignment operator?
src/tests/containerizer/containerizer_tests.cpp (line 60)
<https://reviews.apache.org/r/55761/#comment234069>
2 lines between tests.
src/tests/containerizer/containerizer_tests.cpp (line 62)
<https://reviews.apache.org/r/55761/#comment234080>
s/zero/zero amount/
src/tests/containerizer/containerizer_tests.cpp (lines 75 - 76)
<https://reviews.apache.org/r/55761/#comment234088>
see above.
src/tests/containerizer/containerizer_tests.cpp (lines 76 - 77)
<https://reviews.apache.org/r/55761/#comment234089>
also, can you just use "stringify" here?
src/tests/containerizer/containerizer_tests.cpp (line 79)
<https://reviews.apache.org/r/55761/#comment234070>
2 lines between tests.
src/tests/containerizer/containerizer_tests.cpp (lines 94 - 96)
<https://reviews.apache.org/r/55761/#comment234090>
can you use stringify here?
- Vinod Kone
On Jan. 20, 2017, 1:01 p.m., Bruce Merry wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55761/
> -----------------------------------------------------------
>
> (Updated Jan. 20, 2017, 1:01 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-6821
> https://issues.apache.org/jira/browse/MESOS-6821
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Instead of doing a simple substring search for known resources like
> "cpus" to decide whether they have been explicitly specified, it now
> parses the resource string to check that a resource with that exact
> name was specified.
>
> This splits out a fromString static member function from the start of
> Resources::parse that does parsing using either fromJSON or
> fromSimpleString and makes it available to use in
> mesos::internal::slave::Containerizer::resources.
>
> Fixes MESOS-6821.
>
>
> Diffs
> -----
>
> include/mesos/resources.hpp c2e6d6680d9687f0bbfb73e84ffb1457072c4412
> src/Makefile.am 3d0e82cb522d379923e00e5e98e9c9f646fdd3db
> src/common/resources.cpp be9bca2063e9f0e60c5faa0142077bea56272e45
> src/slave/containerizer/containerizer.cpp d46882baa904fd439bffb23c324828b777228f1c
> src/tests/containerizer/containerizer_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/55761/diff/
>
>
> Testing
> -------
>
> Regression test added.
>
>
> Thanks,
>
> Bruce Merry
>
>
Re: Review Request 55761: Fixed name matching for automatic resources.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55761/#review163376
-----------------------------------------------------------
Ship it!
Ship It!
- Vinod Kone
On Jan. 25, 2017, 7:10 a.m., Bruce Merry wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55761/
> -----------------------------------------------------------
>
> (Updated Jan. 25, 2017, 7:10 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-6821
> https://issues.apache.org/jira/browse/MESOS-6821
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Instead of doing a simple substring search for known resources like
> "cpus" to decide whether they have been explicitly specified, it now
> parses the resource string to check that a resource with that exact
> name was specified.
>
> This splits out a fromString static member function from the start of
> Resources::parse that does parsing using either fromJSON or
> fromSimpleString and makes it available to use in
> mesos::internal::slave::Containerizer::resources.
>
> Fixes MESOS-6821.
>
>
> Diffs
> -----
>
> include/mesos/resources.hpp c2e6d6680d9687f0bbfb73e84ffb1457072c4412
> src/Makefile.am 3d0e82cb522d379923e00e5e98e9c9f646fdd3db
> src/common/resources.cpp be9bca2063e9f0e60c5faa0142077bea56272e45
> src/slave/containerizer/containerizer.cpp d46882baa904fd439bffb23c324828b777228f1c
> src/tests/containerizer/containerizer_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/55761/diff/
>
>
> Testing
> -------
>
> Regression test added.
>
>
> Thanks,
>
> Bruce Merry
>
>
Re: Review Request 55761: Fixed name matching for automatic resources.
Posted by Bruce Merry <bm...@ska.ac.za>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55761/
-----------------------------------------------------------
(Updated Jan. 25, 2017, 7:10 a.m.)
Review request for mesos and Vinod Kone.
Changes
-------
Use stringify in port comparison tests.
Bugs: MESOS-6821
https://issues.apache.org/jira/browse/MESOS-6821
Repository: mesos
Description
-------
Instead of doing a simple substring search for known resources like
"cpus" to decide whether they have been explicitly specified, it now
parses the resource string to check that a resource with that exact
name was specified.
This splits out a fromString static member function from the start of
Resources::parse that does parsing using either fromJSON or
fromSimpleString and makes it available to use in
mesos::internal::slave::Containerizer::resources.
Fixes MESOS-6821.
Diffs (updated)
-----
include/mesos/resources.hpp c2e6d6680d9687f0bbfb73e84ffb1457072c4412
src/Makefile.am 3d0e82cb522d379923e00e5e98e9c9f646fdd3db
src/common/resources.cpp be9bca2063e9f0e60c5faa0142077bea56272e45
src/slave/containerizer/containerizer.cpp d46882baa904fd439bffb23c324828b777228f1c
src/tests/containerizer/containerizer_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/55761/diff/
Testing
-------
Regression test added.
Thanks,
Bruce Merry
Re: Review Request 55761: Fixed name matching for automatic resources.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55761/#review162843
-----------------------------------------------------------
Patch looks great!
Reviews applied: [55761]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On Jan. 24, 2017, 2:59 p.m., Bruce Merry wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55761/
> -----------------------------------------------------------
>
> (Updated Jan. 24, 2017, 2:59 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-6821
> https://issues.apache.org/jira/browse/MESOS-6821
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Instead of doing a simple substring search for known resources like
> "cpus" to decide whether they have been explicitly specified, it now
> parses the resource string to check that a resource with that exact
> name was specified.
>
> This splits out a fromString static member function from the start of
> Resources::parse that does parsing using either fromJSON or
> fromSimpleString and makes it available to use in
> mesos::internal::slave::Containerizer::resources.
>
> Fixes MESOS-6821.
>
>
> Diffs
> -----
>
> include/mesos/resources.hpp c2e6d6680d9687f0bbfb73e84ffb1457072c4412
> src/Makefile.am 3d0e82cb522d379923e00e5e98e9c9f646fdd3db
> src/common/resources.cpp be9bca2063e9f0e60c5faa0142077bea56272e45
> src/slave/containerizer/containerizer.cpp d46882baa904fd439bffb23c324828b777228f1c
> src/tests/containerizer/containerizer_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/55761/diff/
>
>
> Testing
> -------
>
> Regression test added.
>
>
> Thanks,
>
> Bruce Merry
>
>
Re: Review Request 55761: Fixed name matching for automatic resources.
Posted by Bruce Merry <bm...@ska.ac.za>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55761/
-----------------------------------------------------------
(Updated Jan. 24, 2017, 2:59 p.m.)
Review request for mesos and Vinod Kone.
Bugs: MESOS-6821
https://issues.apache.org/jira/browse/MESOS-6821
Repository: mesos
Description
-------
Instead of doing a simple substring search for known resources like
"cpus" to decide whether they have been explicitly specified, it now
parses the resource string to check that a resource with that exact
name was specified.
This splits out a fromString static member function from the start of
Resources::parse that does parsing using either fromJSON or
fromSimpleString and makes it available to use in
mesos::internal::slave::Containerizer::resources.
Fixes MESOS-6821.
Diffs (updated)
-----
include/mesos/resources.hpp c2e6d6680d9687f0bbfb73e84ffb1457072c4412
src/Makefile.am 3d0e82cb522d379923e00e5e98e9c9f646fdd3db
src/common/resources.cpp be9bca2063e9f0e60c5faa0142077bea56272e45
src/slave/containerizer/containerizer.cpp d46882baa904fd439bffb23c324828b777228f1c
src/tests/containerizer/containerizer_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/55761/diff/
Testing
-------
Regression test added.
Thanks,
Bruce Merry
Re: Review Request 55761: Fixed name matching for automatic resources.
Posted by Bruce Merry <bm...@ska.ac.za>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55761/
-----------------------------------------------------------
(Updated Jan. 24, 2017, 2:56 p.m.)
Review request for mesos and Vinod Kone.
Bugs: MESOS-6821
https://issues.apache.org/jira/browse/MESOS-6821
Repository: mesos
Description
-------
Instead of doing a simple substring search for known resources like
"cpus" to decide whether they have been explicitly specified, it now
parses the resource string to check that a resource with that exact
name was specified.
This splits out a fromString static member function from the start of
Resources::parse that does parsing using either fromJSON or
fromSimpleString and makes it available to use in
mesos::internal::slave::Containerizer::resources.
Fixes MESOS-6821.
Diffs (updated)
-----
docs/contributors.yaml 2f8ec11d449dedaee325dda56fadaacebcdeb014
include/mesos/resources.hpp c2e6d6680d9687f0bbfb73e84ffb1457072c4412
src/Makefile.am 3d0e82cb522d379923e00e5e98e9c9f646fdd3db
src/common/resources.cpp be9bca2063e9f0e60c5faa0142077bea56272e45
src/slave/containerizer/containerizer.cpp d46882baa904fd439bffb23c324828b777228f1c
src/slave/containerizer/mesos/launch.cpp fc562fcfb265b4de49cdc5a6c10347268a1ecba2
src/tests/containerizer/containerizer_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/55761/diff/
Testing
-------
Regression test added.
Thanks,
Bruce Merry
Re: Review Request 55761: Fixed name matching for automatic resources.
Posted by Bruce Merry <bm...@ska.ac.za>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55761/#review162414
-----------------------------------------------------------
src/tests/containerizer/containerizer_tests.cpp (line 34)
<https://reviews.apache.org/r/55761/#comment233751>
I wasn't sure what to call this class. ContainerizerTest is already taken for tests templated on the specific containerizer.
- Bruce Merry
On Jan. 20, 2017, 1:01 p.m., Bruce Merry wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55761/
> -----------------------------------------------------------
>
> (Updated Jan. 20, 2017, 1:01 p.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-6821
> https://issues.apache.org/jira/browse/MESOS-6821
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Instead of doing a simple substring search for known resources like
> "cpus" to decide whether they have been explicitly specified, it now
> parses the resource string to check that a resource with that exact
> name was specified.
>
> This splits out a fromString static member function from the start of
> Resources::parse that does parsing using either fromJSON or
> fromSimpleString and makes it available to use in
> mesos::internal::slave::Containerizer::resources.
>
> Fixes MESOS-6821.
>
>
> Diffs
> -----
>
> include/mesos/resources.hpp c2e6d6680d9687f0bbfb73e84ffb1457072c4412
> src/Makefile.am 3d0e82cb522d379923e00e5e98e9c9f646fdd3db
> src/common/resources.cpp be9bca2063e9f0e60c5faa0142077bea56272e45
> src/slave/containerizer/containerizer.cpp d46882baa904fd439bffb23c324828b777228f1c
> src/tests/containerizer/containerizer_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/55761/diff/
>
>
> Testing
> -------
>
> Regression test added.
>
>
> Thanks,
>
> Bruce Merry
>
>