mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vinod Kone <vinodk...@gmail.com>
Subject Re: Review Request 56053: Added a 'SECRET' type to the 'Environment' protobuf message.
Date Tue, 31 Jan 2017 20:31:40 GMT

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




include/mesos/mesos.proto (lines 1909 - 1911)
<https://reviews.apache.org/r/56053/#comment235193>

    This comment should be removed in this patch. It should just say "only one of these must
be set."



include/mesos/v1/mesos.proto (line 1892)
<https://reviews.apache.org/r/56053/#comment235194>

    ditto.



src/common/validation.cpp (lines 92 - 105)
<https://reviews.apache.org/r/56053/#comment235196>

    If type is VALUE, value must be set. For UNKNOWN it's debatable because when new enum
values are added that are not yet known to the validation code (upgrade case), it's not clear
if we should expect `value` to be set.
    
    Maybe we can do so:
    
    ```
    if (type == SECRET)
      `secret` must be set
    
    if (type == VALUE)
      `value` must be set
      
    if (type == UNKNOWN)
       // Add a comment saying this is for backwards compatibility reasons
       `value` must be set
    
    ```



src/tests/slave_validation_tests.cpp (line 140)
<https://reviews.apache.org/r/56053/#comment235197>

    I don't think we want to remove this constraint. See above.


- Vinod Kone


On Jan. 29, 2017, 5:10 a.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56053/
> -----------------------------------------------------------
> 
> (Updated Jan. 29, 2017, 5:10 a.m.)
> 
> 
> Review request for mesos, Jan Schlicht and Vinod Kone.
> 
> 
> Bugs: MESOS-7009
>     https://issues.apache.org/jira/browse/MESOS-7009
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch adds a field of type `Secret` to the
> `Environment` protobuf message, enabling the passing
> of secrets into the environments of executors and
> tasks. Additional validation and test code is added
> as well.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto a08b8fb2edf26ae730467557ae0e33e3d4252593 
>   include/mesos/v1/mesos.proto 4a4609e7db659252ef7f1ca61f8ba079b901b7d0 
>   src/common/validation.cpp b2548ad87b4227d6e498c49b5694acb362f6281b 
>   src/tests/slave_validation_tests.cpp 5de771114982751e7796f55dcacd4384c6989efb 
> 
> Diff: https://reviews.apache.org/r/56053/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


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