-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72839/#review221836
-----------------------------------------------------------


Ship it!




Hm.. I was wondering how we exposed the suppressed roles in /frameworks & 
GET_FRAMEWORKS, since I didn't see them in the interfaces here, but looks like 
we don't expose them?

An alternative would be to include this in the allocatorOptions, like 
suppressed roles, but store allocatorOptions in the master's Framework struct, 
rather than throwing it away after passing it through to the allocator. Seems a 
bit cleaner and won't require suppressed roles to also get added to all these 
interfaces? I guess you perhaps didn't go this route since allocator options is 
storing the compiled form of this information, and the allocator does not need 
the raw form?


src/master/master.hpp
Lines 2547 (patched)
<https://reviews.apache.org/r/72839/#comment310915>

    { on newline



src/master/master.cpp
Lines 3261-3275 (original), 3297-3313 (patched)
<https://reviews.apache.org/r/72839/#comment310916>

    Maybe just touch the protobuf once, then work off the option? Looks a bit 
clearer?
    
    ```
    Option<OfferConstraints> offerConstraints;
    if (call.has_offer_constraints()) {
      offerConstraints = std::move(*call.mutable_offer_constraints());
    }
    
    allocator::FrameworkOptions allocatorOptions;
    if (offerConstraints.isSome()) {
      // TODO(asekretenko): Validate roles in offer constraints (see 
MESOS-10176).
      Try<OfferConstraintsFilter> filter = OfferConstraintsFilter::create(
          offerConstraintsFilterOptions, *offerConstraints);
    
      if (filter.isError()) {
        return process::http::BadRequest(
            "'UpdateFramework.offer_constraints' are not valid: " +
            filter.error());
      }
    
      allocatorOptions.offerConstraintsFilter = std::move(*filter);
    }
    ```


- Benjamin Mahler


On Sept. 7, 2020, 3:26 p.m., Andrei Sekretenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72839/
> -----------------------------------------------------------
> 
> (Updated Sept. 7, 2020, 3:26 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-10179
>     https://issues.apache.org/jira/browse/MESOS-10179
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This is a prerequisite for exposing framework's offer constraints
> via master API endpoints.
> 
> 
> Diffs
> -----
> 
>   src/master/framework.cpp e9e2d20bdc89dd299b2376dd2f67ae056538c3e5 
>   src/master/master.hpp 3d1d4723d1bcef1880404007410bf00656399f10 
>   src/master/master.cpp 5b5d5c359ca48eb0b4f5554fe7c91e928d4da08d 
> 
> 
> Diff: https://reviews.apache.org/r/72839/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>

Reply via email to