> On Feb. 13, 2015, 2:27 p.m., Alexander Rukletsov wrote:
> > Let's create a special test case for filters. It looks like following test
> > cases are uncovered (or I haven't noticed them in the code):
> > 1. `Resources::Filter::unreserved()` and `Resources::Filter::reserved("*")`
> > are essentially the same.
> > 2. `Resources::Filter::any()` does what it should.
> > 3. `Resources::Filter(predicate)` filters resources according to the given
> > predicate.
These tests currently exist in `ResourcesTest.Reservations` although they're
not exactly filter-specific. Regarding the cases:
1. It does test the difference between `unreserved` and `reserved("*")`.
These are different because they actually don't convey the same meaning.
Resources are not reserved for "*", but rather are unreserved. The
distinction here is that **unreserved** resources can be offered to any
framework whereas resources that are **reserved for "*"** (if this was actually
a thing) could only be offered to frameworks with role="*". The same way
resources reserved for R can only be offered to frameworks with role=R. Since
"*" is not simply a role name, but actually convey different meaning, we've
distinguished the different meaning into these functions.
2. I've removed the predefined `any` filter, since it's only used in one
place and is largely useless in every other situation. This is exactly when we
want a inline filter definition :)
3. There are test cases that test these as well, but I should probably add
more.
> On Feb. 13, 2015, 2:27 p.m., Alexander Rukletsov wrote:
> > src/common/resources.cpp, line 559
> > <https://reviews.apache.org/r/30911/diff/1/?file=861441#file861441line559>
> >
> > I'm not sure we can use `initializer_list`. Have we dropped the support
> > for archaic gcc already?
`initializer_list` is available in GCC-4.4:
https://gcc.gnu.org/gcc-4.4/cxx0x_status.html
Although I do need to wait for https://reviews.apache.org/r/29497/ to get
committed technically.
> On Feb. 13, 2015, 2:27 p.m., Alexander Rukletsov wrote:
> > include/mesos/resources.hpp, line 281
> > <https://reviews.apache.org/r/30911/diff/1/?file=861440#file861440line281>
> >
> > Does it make sense to provide `static Filter predicate(const Predicate&
> > p)` for consistency and make this c-tor private?
The API of the `filter` has changed slightly. I spoke with Jie yesterday on the
interface and he mentioned that the callsites are ugly to parse with the parens
(they were static functions that returned `Filter` objects before). So `filter`
is now a non-member function, and rather than composing filters themselves
we'll compose the predicates and call `filter` with it instead. Please take a
look how it looks now and provide your feedback! Thanks Alex!
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30911/#review72362
-----------------------------------------------------------
On Feb. 14, 2015, 12:11 a.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30911/
> -----------------------------------------------------------
>
> (Updated Feb. 14, 2015, 12:11 a.m.)
>
>
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, and
> Vinod Kone.
>
>
> Bugs: MESOS-2348
> https://issues.apache.org/jira/browse/MESOS-2348
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See [JIRA Ticket](https://issues.apache.org/jira/browse/MESOS-2348).
>
>
> Diffs
> -----
>
> include/mesos/resources.hpp c7cc46e0183ea97013dd088a717da6c0e6ed5cf0
> src/common/resources.cpp 98371f6873482d0cdbefeb279b58ae6cc680a88f
> src/master/allocator/mesos/hierarchical.hpp
> 2680d6231927867d5a8d75cbc42b81d6c75fc7f2
> src/master/master.hpp 6a39df04514c756415354fae66c5835ada191c52
> src/master/validation.cpp acc35b25c93f2d3900d79c3070b1d681416ed66b
> src/slave/slave.cpp ec7ec1356e745bb07484ae1755c9183b038043b3
> src/tests/hierarchical_allocator_tests.cpp
> eeecfb64540b16666915074aaffaa5d506b203bc
> src/tests/resources_tests.cpp 3f98782fd437dba808d720bf8e9b94b8fa7e0feb
>
> Diff: https://reviews.apache.org/r/30911/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Michael Park
>
>
