mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Bannier <benjamin.bann...@mesosphere.io>
Subject Re: Review Request 66309: Externalized creation of resource provider manager backing storage.
Date Thu, 19 Apr 2018 09:20:21 GMT


> On April 18, 2018, 11:51 p.m., Chun-Hung Hsiao wrote:
> > src/resource_provider/registrar.hpp
> > Lines 67-73 (original), 69-75 (patched)
> > <https://reviews.apache.org/r/66309/diff/3/?file=1995440#file1995440line69>
> >
> >     Not yours, but if each `create()` corresponds to a different type of registrar,
how about moving these functions into their corresponding `MasterRegistrar` and `AgentRegistrar`?
> >     
> >     I guess it depends on how meanful it is to encapsulate the actual type of registrar
from the callers.
> 
> Chun-Hung Hsiao wrote:
>     Also, if there is no need to encapsulate the actual type of registrar, then it is
meanleass to have these `create()` functions, since we don't do extra checks and return errors.

I think having these factory functions still adds value as they hide away non-trivial details.
 The arguments pretty clearly communicate how they can be used, e.g., in the master we would
not be able to transfer ownership of the underlying storage leaving only one possible factory
to use.

Let's keep this as is, dropping.


- Benjamin


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


On April 19, 2018, 11:19 a.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66309/
> -----------------------------------------------------------
> 
> (Updated April 19, 2018, 11:19 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-8735
>     https://issues.apache.org/jira/browse/MESOS-8735
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch changes the way the storage backing an agent's resource
> provider registrar is created: while before we created it implicitly
> when constructing the registrar, we now consume storage passed on
> construction.
> 
> Being able to explicitly inject the used storage simplifies testing.
> 
> 
> Diffs
> -----
> 
>   src/resource_provider/registrar.hpp 39f45b0a2a7c35bfe72a9305f5248409e4a3ef45 
>   src/resource_provider/registrar.cpp 92ef9aecb1e93d10f46e53984391558f9901a60b 
>   src/tests/resource_provider_manager_tests.cpp c52541bf130ccf4795b989b53331176a64a097ea

> 
> 
> Diff: https://reviews.apache.org/r/66309/diff/4/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


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