mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Mahler <bmah...@apache.org>
Subject Re: Review Request 55970: Updated the master's HTTP operations to handle MULTI_ROLE changes.
Date Sat, 04 Feb 2017 03:02:31 GMT


> On Jan. 28, 2017, 3:25 p.m., Guangya Liu wrote:
> > src/master/validation.cpp, lines 1654-1667
> > <https://reviews.apache.org/r/55970/diff/1/?file=1616023#file1616023line1654>
> >
> >     Did not quite catch up the `TODO` here, can you please elaborate for why want
such a function?

We have a few transformation functions:

`flatten()` which is a bit unintuitive,
`createStrippedScalarQuantity()` which makes some decisions about what to strip

It would be nice if we had a pattern for applying transformations that made it clearer what
we're stripping. I'm just pointing out that unallocate falls under ths same pattern.

I was hesitant to pull out `unallocated()` because our current naming convention is that past-tense
verbs peform filtering. So a `Resources::unallocated()` would just return those resources
that are unallocated rather than a copy with the allocation stripped. I think we need something
like `resources::transformations::unallocated()` or `Resources::transformed()`.


> On Jan. 28, 2017, 3:25 p.m., Guangya Liu wrote:
> > src/master/quota_handler.cpp, lines 196-200
> > <https://reviews.apache.org/r/55970/diff/1/?file=1616022#file1616022line196>
> >
> >     Per the comments in /r/55870/ , how about move this to resources_utils.cpp?

I would like to pull something shared out for this, but see my comment below for why I held
off for now.


> On Jan. 28, 2017, 3:25 p.m., Guangya Liu wrote:
> > src/master/http.cpp, lines 4614-4615
> > <https://reviews.apache.org/r/55970/diff/1/?file=1616021#file1616021line4614>
> >
> >     I think this can be replaced by the util function in below comments?

Hm.. I didn't quite follow your suggestion here.


- Benjamin


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


On Jan. 26, 2017, 1:08 a.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55970/
> -----------------------------------------------------------
> 
> (Updated Jan. 26, 2017, 1:08 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> With the addition of MULTI_ROLE framework support, allocated
> resources in the master have `Resource.AllocationInfo` set. This
> means that the master's http endpoints that apply operations or
> perform checks between unallocated and allocated resources must
> be updated to continue to function correctly.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp a44621f39cb059e654a56f57f75b38947f3a4230 
>   src/master/quota_handler.cpp 6e6e7375219d34e6e8d011a025b5f5d70b87383b 
>   src/master/validation.cpp 5f134b781901f2de6a90fa6a10d42cc7629c27a0 
> 
> Diff: https://reviews.apache.org/r/55970/diff/
> 
> 
> Testing
> -------
> 
> The existing tests pass at the end of this review chain.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>


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