mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Bannier <bbann...@apache.org>
Subject Re: Review Request 63480: Added the initial implementation for applying offer operations.
Date Mon, 06 Nov 2017 16:12:49 GMT

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


Fix it, then Ship it!





src/resource_provider/manager.cpp
Lines 165 (patched)
<https://reviews.apache.org/r/63480/#comment267442>

    There is no need to use an `Owned` here; a `ResourceProvider` would work just fine.
    
    The mutating `update*` functions should just take a `ResourceProvider*` in that case.
They should do this even if we'd store an `Owned` here since they do not do anything with
the passed provider's `Owned`'ness.



src/resource_provider/manager.cpp
Lines 348-349 (patched)
<https://reviews.apache.org/r/63480/#comment267448>

    Does it make sense to just drop this message with some logging instead of failing over
the agent?



src/resource_provider/manager.cpp
Line 337 (original), 413 (patched)
<https://reviews.apache.org/r/63480/#comment267443>

    We should just take a `ResourceProvider*` here; these functions do not care about `resourceProvider`'s
`Owned`'ness.



src/resource_provider/manager.cpp
Line 345 (original), 421 (patched)
<https://reviews.apache.org/r/63480/#comment267444>

    We should just take a `ResourceProvider*` here; these functions do not care about `resourceProvider`'s
`Owned`'ness.



src/resource_provider/manager.cpp
Lines 482 (patched)
<https://reviews.apache.org/r/63480/#comment267445>

    Not sure if this function should be `const` as it maps a non-`const` process function.


- Benjamin Bannier


On Nov. 2, 2017, 12:14 a.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63480/
> -----------------------------------------------------------
> 
> (Updated Nov. 2, 2017, 12:14 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Gaston Kleiman, Greg Mann,
and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The resource provider manager provides an `applyOfferOperation` method
> for offer operation affecting resource providers. The resources on
> which the operation should be applied contains a resource provider ID.
> This will be extracted and an event will be sent to the respective
> resource provider.
> 
> (This is based on https://reviews.apache.org/r/61810)
> 
> 
> Diffs
> -----
> 
>   src/messages/messages.proto 9dfe7f07c2e1171be17f1bd7b6a54a80410c44f6 
>   src/resource_provider/manager.hpp 3b70e75c6b6721864ae0ee9c4a593b5035d8388f 
>   src/resource_provider/manager.cpp 11f890156f0fd099f8a97b07cdc458a0726ee78e 
>   src/slave/slave.cpp 817dca4fc3e1028ccc13b8a1bb1ee783afb77166 
> 
> 
> Diff: https://reviews.apache.org/r/63480/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


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