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 64168: Introduced a 'UUID' type.
Date Mon, 11 Dec 2017 16:10:11 GMT

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



I still see a few resource version-related UUIDs in `messages.proto` and `resource_provider.proto`;
could you replace these as well?


src/common/type_utils.cpp
Lines 798 (patched)
<https://reviews.apache.org/r/64168/#comment271953>

    I think putting a `CHECK` here is not a good idea; users might e.g., attempt to print
an invalid `UUID` (which is just a bag of `bytes`).
    
    I think we should either use a representation that always works (e.g., base64-encoding),
or maybe even better print explicitly catch the error case here by e.g., printing `INVALID
UUID` or similar if no `id::UUID` can be decoded.



src/messages/messages.cpp
Line 91 (original), 91 (patched)
<https://reviews.apache.org/r/64168/#comment271954>

    Let's just add `operator==` and `operator!=` for `UUID` in both v0 and v1.


- Benjamin Bannier


On Dec. 8, 2017, 4:15 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64168/
> -----------------------------------------------------------
> 
> (Updated Dec. 8, 2017, 4:15 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8309
>     https://issues.apache.org/jira/browse/MESOS-8309
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Introduced a 'UUID' type.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 839ddb1cb41471d36423a2fc149acf90b973d413 
>   include/mesos/resource_provider/resource_provider.proto 360d8620dd1858321f24daf52eb7bee9742427a5

>   include/mesos/type_utils.hpp 506f667073df9c4368354d07dcc012e5e0dd36f1 
>   include/mesos/v1/mesos.proto 0ea17a83705774a69a301599c9c905e08cdef4d1 
>   include/mesos/v1/resource_provider/resource_provider.proto 3e799f305cbd3a7d3f94186ee1486e040e6ad1b6

>   src/common/protobuf_utils.cpp c5504a00a7a84b824743f00c6b97ea299ac66eb4 
>   src/common/type_utils.cpp 65586a57b6d384350673903fb399b89a83fb9c37 
>   src/master/master.hpp 1f5daae6abbf81cd5fc0341f5ee6c9678beb0d64 
>   src/master/master.cpp 5cba50636a9351d29660c54fad7734fcfea547b9 
>   src/messages/messages.cpp 56876f2d5a92237ccb85ec6d8ab8eb872061a7b5 
>   src/messages/messages.proto f71178438660fa16aad8f290391baba7813fbff0 
>   src/resource_provider/manager.cpp f98611c351f11b15281dd3ef673ef80db1c44f07 
>   src/resource_provider/message.hpp bbf6bb2a0a4aa901ac038cdbe4dd46acc0f2e453 
>   src/slave/slave.cpp 54d8bcc035227dd6896ffa6e692a91749c0b56a6 
>   src/status_update_manager/offer_operation.cpp f66690eadc0d7b16cc7de1b518904db2cab7ef82

>   src/tests/mesos.hpp 95068895da1f7267d4b326c636f29a3a68177105 
>   src/tests/offer_operation_status_update_manager_tests.cpp a31a525e1ee05492861885f59123471bc4ef33c6

>   src/tests/resource_provider_manager_tests.cpp a6eb4c9a303780029244e069bdf550a8cd9c7bb4

>   src/tests/resource_provider_validation_tests.cpp bf789a05771b7c25f2fc2a8a5b35d38519e4793b

>   src/tests/slave_tests.cpp 07145432c35c85a755b66ba575c2a46db072ce5c 
> 
> 
> Diff: https://reviews.apache.org/r/64168/diff/4/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>


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