You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Bannier <be...@mesosphere.io> on 2018/02/09 17:27:40 UTC
Review Request 65588: Used proto UUID instead stout UUID internally
for operation IDs.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65588/
-----------------------------------------------------------
Review request for mesos, Jie Yu and Jan Schlicht.
Bugs: MESOS-8382
https://issues.apache.org/jira/browse/MESOS-8382
Repository: mesos
Description
-------
Used proto UUID instead stout UUID internally for operation IDs.
Diffs
-----
include/mesos/resource_provider/resource_provider.proto e96a40493c351a7242465c8591cae981abc92f24
src/common/protobuf_utils.hpp 9a940db188436b9c5a39b0637cb4c15ee2ab5266
src/common/protobuf_utils.cpp b5c2997ada8362e42150fa3cfd762120e5ea715f
src/master/master.hpp b434d2398b8815811345b6586ca586d2025cb2a2
src/master/master.cpp d7d22866f7a4eb87bd8949efafc97e828e7d4b94
src/resource_provider/manager.cpp cc195a3d35b93dd6493951de1ff8a1cb8a886377
src/resource_provider/message.hpp 2511af611edc94e3e3a78fce475bc8cd85ffc3f9
src/slave/slave.hpp 30151c4886e12e9183a971b86b854e28a8ca1b39
src/slave/slave.cpp f98f37321872d090176b7cc50873fc3c627773f5
Diff: https://reviews.apache.org/r/65588/diff/1/
Testing
-------
`make check`
Thanks,
Benjamin Bannier
Re: Review Request 65588: Used proto UUID instead stout UUID
internally for operation IDs.
Posted by Benjamin Bannier <be...@mesosphere.io>.
> On Feb. 13, 2018, 10:52 a.m., Jan Schlicht wrote:
> > src/common/protobuf_utils.cpp
> > Line 467 (original), 467 (patched)
> > <https://reviews.apache.org/r/65588/diff/1/?file=1955477#file1955477line467>
> >
> > How about we create a helper function `mesos::UUID createRandomUUID()` in `protobuf_utils` even if that function would be almost trivial? This would allow us to have the expectations of a UUID (i.e. it's `UUID::random().toBytes()`, not `UUID::random().toString()`) in a single place.
> >
> > Here and below, where `->set_value(id::UUID::random().toBytes())` is used.
Good idea, I created a function `mesos::UUID createUUID(const Option<id::UUID>& = None())` in https://reviews.apache.org/r/65674/ performing this work.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65588/#review197394
-----------------------------------------------------------
On Feb. 15, 2018, 3:54 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65588/
> -----------------------------------------------------------
>
> (Updated Feb. 15, 2018, 3:54 p.m.)
>
>
> Review request for mesos, Jie Yu and Jan Schlicht.
>
>
> Bugs: MESOS-8382
> https://issues.apache.org/jira/browse/MESOS-8382
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Used proto UUID instead stout UUID internally for operation IDs.
>
>
> Diffs
> -----
>
> include/mesos/resource_provider/resource_provider.proto e96a40493c351a7242465c8591cae981abc92f24
> src/common/protobuf_utils.hpp 9a940db188436b9c5a39b0637cb4c15ee2ab5266
> src/common/protobuf_utils.cpp b5c2997ada8362e42150fa3cfd762120e5ea715f
> src/master/master.hpp c4d3c8080dc7899968b1030696172ed73d473bc1
> src/master/master.cpp b06d7a6e2fbbb81b97eaf537d5b6745c73dc867d
> src/resource_provider/manager.cpp cc195a3d35b93dd6493951de1ff8a1cb8a886377
> src/resource_provider/message.hpp 2511af611edc94e3e3a78fce475bc8cd85ffc3f9
> src/slave/slave.hpp 42c3ebcf52b608e40e26ca85f5efb054dad2d2a1
> src/slave/slave.cpp c5ec62c0e55e7416d9cd2a49c13459b85e315150
>
>
> Diff: https://reviews.apache.org/r/65588/diff/2/
>
>
> Testing
> -------
>
> `make check`
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 65588: Used proto UUID instead stout UUID
internally for operation IDs.
Posted by Jan Schlicht <ja...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65588/#review197394
-----------------------------------------------------------
Thanks for cleaning this up!
Looks great, only minor thing that I've noticed and which isn't in the scope of this diff is some inconsistent naming in `slave.cpp`. E.g., there's `operationUUID` vs. `operationUuid`, especially in `Slave::handleResourceProviderMessage`. It would be great I we could agree on a convention here.
src/common/protobuf_utils.cpp
Line 467 (original), 467 (patched)
<https://reviews.apache.org/r/65588/#comment277521>
How about we create a helper function `mesos::UUID createRandomUUID()` in `protobuf_utils` even if that function would be almost trivial? This would allow us to have the expectations of a UUID (i.e. it's `UUID::random().toBytes()`, not `UUID::random().toString()`) in a single place.
Here and below, where `->set_value(id::UUID::random().toBytes())` is used.
- Jan Schlicht
On Feb. 9, 2018, 6:27 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65588/
> -----------------------------------------------------------
>
> (Updated Feb. 9, 2018, 6:27 p.m.)
>
>
> Review request for mesos, Jie Yu and Jan Schlicht.
>
>
> Bugs: MESOS-8382
> https://issues.apache.org/jira/browse/MESOS-8382
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Used proto UUID instead stout UUID internally for operation IDs.
>
>
> Diffs
> -----
>
> include/mesos/resource_provider/resource_provider.proto e96a40493c351a7242465c8591cae981abc92f24
> src/common/protobuf_utils.hpp 9a940db188436b9c5a39b0637cb4c15ee2ab5266
> src/common/protobuf_utils.cpp b5c2997ada8362e42150fa3cfd762120e5ea715f
> src/master/master.hpp b434d2398b8815811345b6586ca586d2025cb2a2
> src/master/master.cpp d7d22866f7a4eb87bd8949efafc97e828e7d4b94
> src/resource_provider/manager.cpp cc195a3d35b93dd6493951de1ff8a1cb8a886377
> src/resource_provider/message.hpp 2511af611edc94e3e3a78fce475bc8cd85ffc3f9
> src/slave/slave.hpp 30151c4886e12e9183a971b86b854e28a8ca1b39
> src/slave/slave.cpp f98f37321872d090176b7cc50873fc3c627773f5
>
>
> Diff: https://reviews.apache.org/r/65588/diff/1/
>
>
> Testing
> -------
>
> `make check`
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 65588: Used proto UUID instead stout UUID
internally for operation IDs.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65588/#review198657
-----------------------------------------------------------
Ship it!
Ship It!
- Greg Mann
On Feb. 23, 2018, 3:22 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65588/
> -----------------------------------------------------------
>
> (Updated Feb. 23, 2018, 3:22 p.m.)
>
>
> Review request for mesos, Jie Yu and Jan Schlicht.
>
>
> Bugs: MESOS-8382
> https://issues.apache.org/jira/browse/MESOS-8382
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Used proto UUID instead stout UUID internally for operation IDs.
>
>
> Diffs
> -----
>
> include/mesos/resource_provider/resource_provider.proto e96a40493c351a7242465c8591cae981abc92f24
> src/common/protobuf_utils.hpp 9a940db188436b9c5a39b0637cb4c15ee2ab5266
> src/common/protobuf_utils.cpp f516091e9208552488c154f34adde6f20d3413bf
> src/master/master.hpp 1fadbe61f774e8ca9926af4e39b3d6c8e24fe5df
> src/master/master.cpp 9cea7bb6a6ee8bf5f4226d07111bcfa6f5d3a88c
> src/resource_provider/manager.cpp cc195a3d35b93dd6493951de1ff8a1cb8a886377
> src/resource_provider/message.hpp 2511af611edc94e3e3a78fce475bc8cd85ffc3f9
> src/resource_provider/storage/provider.cpp 63dde512fd8cc9f68f5f48a96869eb09b23b6f4a
> src/slave/slave.hpp 42c3ebcf52b608e40e26ca85f5efb054dad2d2a1
> src/slave/slave.cpp e2facb3c15a2f907f6497c58a36842ed707f2c70
>
>
> Diff: https://reviews.apache.org/r/65588/diff/6/
>
>
> Testing
> -------
>
> `make check`
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 65588: Used proto UUID instead stout UUID
internally for operation IDs.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65588/
-----------------------------------------------------------
(Updated Feb. 23, 2018, 4:22 p.m.)
Review request for mesos, Jie Yu and Jan Schlicht.
Changes
-------
* fixed grpc-enabled code
Bugs: MESOS-8382
https://issues.apache.org/jira/browse/MESOS-8382
Repository: mesos
Description
-------
Used proto UUID instead stout UUID internally for operation IDs.
Diffs (updated)
-----
include/mesos/resource_provider/resource_provider.proto e96a40493c351a7242465c8591cae981abc92f24
src/common/protobuf_utils.hpp 9a940db188436b9c5a39b0637cb4c15ee2ab5266
src/common/protobuf_utils.cpp f516091e9208552488c154f34adde6f20d3413bf
src/master/master.hpp 6569c9ea0677f193ec56c276e6deea3f2fe86514
src/master/master.cpp e7d5ac67a94f927bb747bd283a3bf9671b2b8f44
src/resource_provider/manager.cpp cc195a3d35b93dd6493951de1ff8a1cb8a886377
src/resource_provider/message.hpp 2511af611edc94e3e3a78fce475bc8cd85ffc3f9
src/resource_provider/storage/provider.cpp 33abc0e05a804969ae14da9cb9c58698ba1aaea5
src/slave/slave.hpp 42c3ebcf52b608e40e26ca85f5efb054dad2d2a1
src/slave/slave.cpp e2facb3c15a2f907f6497c58a36842ed707f2c70
Diff: https://reviews.apache.org/r/65588/diff/5/
Changes: https://reviews.apache.org/r/65588/diff/4-5/
Testing
-------
`make check`
Thanks,
Benjamin Bannier
Re: Review Request 65588: Used proto UUID instead stout UUID
internally for operation IDs.
Posted by Jan Schlicht <ja...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65588/#review198116
-----------------------------------------------------------
Ship it!
Ship It!
- Jan Schlicht
On Feb. 15, 2018, 3:54 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65588/
> -----------------------------------------------------------
>
> (Updated Feb. 15, 2018, 3:54 p.m.)
>
>
> Review request for mesos, Jie Yu and Jan Schlicht.
>
>
> Bugs: MESOS-8382
> https://issues.apache.org/jira/browse/MESOS-8382
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Used proto UUID instead stout UUID internally for operation IDs.
>
>
> Diffs
> -----
>
> include/mesos/resource_provider/resource_provider.proto e96a40493c351a7242465c8591cae981abc92f24
> src/common/protobuf_utils.hpp 9a940db188436b9c5a39b0637cb4c15ee2ab5266
> src/common/protobuf_utils.cpp f516091e9208552488c154f34adde6f20d3413bf
> src/master/master.hpp 6569c9ea0677f193ec56c276e6deea3f2fe86514
> src/master/master.cpp e7d5ac67a94f927bb747bd283a3bf9671b2b8f44
> src/resource_provider/manager.cpp cc195a3d35b93dd6493951de1ff8a1cb8a886377
> src/resource_provider/message.hpp 2511af611edc94e3e3a78fce475bc8cd85ffc3f9
> src/slave/slave.hpp 42c3ebcf52b608e40e26ca85f5efb054dad2d2a1
> src/slave/slave.cpp e2facb3c15a2f907f6497c58a36842ed707f2c70
>
>
> Diff: https://reviews.apache.org/r/65588/diff/4/
>
>
> Testing
> -------
>
> `make check`
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 65588: Used proto UUID instead stout UUID
internally for operation IDs.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65588/
-----------------------------------------------------------
(Updated Feb. 15, 2018, 3:54 p.m.)
Review request for mesos, Jie Yu and Jan Schlicht.
Changes
-------
Rebased.
Bugs: MESOS-8382
https://issues.apache.org/jira/browse/MESOS-8382
Repository: mesos
Description
-------
Used proto UUID instead stout UUID internally for operation IDs.
Diffs (updated)
-----
include/mesos/resource_provider/resource_provider.proto e96a40493c351a7242465c8591cae981abc92f24
src/common/protobuf_utils.hpp 9a940db188436b9c5a39b0637cb4c15ee2ab5266
src/common/protobuf_utils.cpp b5c2997ada8362e42150fa3cfd762120e5ea715f
src/master/master.hpp c4d3c8080dc7899968b1030696172ed73d473bc1
src/master/master.cpp b06d7a6e2fbbb81b97eaf537d5b6745c73dc867d
src/resource_provider/manager.cpp cc195a3d35b93dd6493951de1ff8a1cb8a886377
src/resource_provider/message.hpp 2511af611edc94e3e3a78fce475bc8cd85ffc3f9
src/slave/slave.hpp 42c3ebcf52b608e40e26ca85f5efb054dad2d2a1
src/slave/slave.cpp c5ec62c0e55e7416d9cd2a49c13459b85e315150
Diff: https://reviews.apache.org/r/65588/diff/2/
Changes: https://reviews.apache.org/r/65588/diff/1-2/
Testing
-------
`make check`
Thanks,
Benjamin Bannier