-----------------------------------------------------------
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
> 
>

Reply via email to