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 70533: Implemented the UPDATE_FRAMEWORK call.
Date Wed, 24 Apr 2019 19:52:42 GMT

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




src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp
Lines 687-691 (patched)
<https://reviews.apache.org/r/70533/#comment301133>

    Shouldn't we just include this in the same case block above as the other calls that aren't
supported in v0? Why log it differently?



src/master/master.cpp
Lines 3206 (patched)
<https://reviews.apache.org/r/70533/#comment301137>

    `std::move(call)` ?



src/master/master.cpp
Lines 3212 (patched)
<https://reviews.apache.org/r/70533/#comment301138>

    `const Future<bool>&` ?



src/master/master.cpp
Lines 3215-3216 (patched)
<https://reviews.apache.org/r/70533/#comment301139>

    should be 2 space indents within a new scope



src/master/master.cpp
Lines 3220-3223 (patched)
<https://reviews.apache.org/r/70533/#comment301140>

    Hm.. perhaps a TODO here for us to be able to know which roles the framework was not authorized
to subscribe to so that we can print them?



src/master/master.cpp
Lines 3229 (patched)
<https://reviews.apache.org/r/70533/#comment301141>

    Right now some of the messages are including periods and some are not, it seems simpler
not to include them



src/master/master.cpp
Lines 3232-3235 (patched)
<https://reviews.apache.org/r/70533/#comment301142>

    no std:: prefix in the .cpp file



src/master/master.cpp
Lines 3232-3235 (patched)
<https://reviews.apache.org/r/70533/#comment301143>

    Can you std::move them out from the protobuf into the std::set to avoid the copying?



src/master/master.cpp
Lines 3237 (patched)
<https://reviews.apache.org/r/70533/#comment301144>

    What does updateFramework do if the changes are not allowed? Crash?
    
    Seems like we should be CHECKing that it succeeded or something..


- Benjamin Mahler


On April 23, 2019, 7:06 p.m., Andrei Sekretenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70533/
> -----------------------------------------------------------
> 
> (Updated April 23, 2019, 7:06 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7258
>     https://issues.apache.org/jira/browse/MESOS-7258
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Implemented UPDATE_FRAMEWORK scheduler call.
> 
> This call allows to perform the same changes in FrameworkInfo as resubscribing a framework.
> 
> HTTP return codes specific to this call:
> 200 OK on success
> 400 Bad request when the requested update is not valid.
> 403 Unathorized when the framework would not be authorized to use some entities (currently
roles) after the requested update.
> 409 Conflict when the framework is removed by a concurrent call.
> 
> No "incomplete updates" occur when an update is invalid. I.e. the update either succeeds
or fails completely.
> 
> An attempt to change framework's user/checkpointing ability is not swallowed silently,
but is treated as an error - this is different from attempting to change these properties
by resubscribing a framework.
> 
> 
> Diffs
> -----
> 
>   src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp 35a04a3d0183697f93dadb3cbfead3ee0c2fea08

>   src/master/http.cpp e7a92d0f554ba4cafaee5a75f09b46eb1bf4a310 
>   src/master/master.hpp 94891af9deeaddb3333fc9d6eabb243aed97f7b7 
>   src/master/master.cpp ad54ae217863a08f4e6d743b39c176b171353084 
>   src/master/validation.cpp f032a781608857d0c9cfa220dd8d70f74d60f1ec 
> 
> 
> Diff: https://reviews.apache.org/r/70533/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>


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