mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vinod Kone <vinodk...@gmail.com>
Subject Re: Review Request 55761: Fixed name matching for automatic resources.
Date Tue, 24 Jan 2017 22:06:31 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message