mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Anand Mazumdar <an...@apache.org>
Subject Re: Review Request 48094: Implemented GET_ROLES Call in v1 master API.
Date Sun, 19 Jun 2016 04:01:33 GMT


> On June 18, 2016, 2:43 p.m., haosdent huang wrote:
> > src/tests/api_tests.cpp, line 456
> > <https://reviews.apache.org/r/48094/diff/5/?file=1423341#file1423341line456>
> >
> >     I think we could not gruantee the `v1Response->get_roles().roles(2)` equal
to `role2`. How about `foreach` here and find the role that name is `role2`?
> 
> Abhishek Dasgupta wrote:
>     roles are getting inserted as per the roleList which is set<string> . So, role2
comes in v1Response->get_roles().roles(2) .. Am I missing something here??
> 
> haosdent huang wrote:
>     I see, but if we don't document this implmentation details in our document/proto,
we could not guarantee the order of roles here. Suppose we change the roles order alphabetcially
in the implementation, this line would be break. Let's avoid to write fragile codes.

Haosdent, we are not advertising the implementation details to the user. The idea is to check
the functional correctness of our implementation without being too verbose. Since, we know
that the `roleList` here is a `set`, we can _assume_ the order in tests.

We already follow this pattern for some of the existing tests, e.g., https://github.com/apache/mesos/blob/master/src/tests/role_tests.cpp#L434


- Anand


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


On June 18, 2016, 4:21 p.m., Abhishek Dasgupta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48094/
> -----------------------------------------------------------
> 
> (Updated June 18, 2016, 4:21 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5494
>     https://issues.apache.org/jira/browse/MESOS-5494
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Implemented GET_ROLES Call in v1 master API.
> 
> 
> Diffs
> -----
> 
>   include/mesos/master/master.proto fa92240 
>   include/mesos/mesos.proto e4c5bd3 
>   include/mesos/v1/master/master.proto 59e978f 
>   include/mesos/v1/mesos.proto 9be22f0 
>   src/master/http.cpp a6beb17 
>   src/master/master.hpp 618d928 
>   src/tests/api_tests.cpp afa5ffa 
> 
> Diff: https://reviews.apache.org/r/48094/diff/
> 
> 
> Testing
> -------
> 
> On Ubuntu 16.04:
> 
> sudo GTEST_FILTER="*MasterAPITest.*" make -j2 check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>


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