mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jie Yu" <yujie....@gmail.com>
Subject Re: Review Request 31162: Backout change 30300 (Move internal protos from mesos::internal to mesos namespace)
Date Wed, 18 Feb 2015 22:46:54 GMT


> On Feb. 18, 2015, 10:42 p.m., Jie Yu wrote:
> > include/mesos/authentication/authentication.hpp, lines 25-27
> > <https://reviews.apache.org/r/31162/diff/1/?file=868264#file868264line25>
> >
> >     We don't do "using namespace" in headers.
> 
> Kapil Arya wrote:
>     Yes, but this was to avoid changing numerous cpp files to include `using namespace`.
 Is it possible to make an exception until we can figure out a better way to handle `internal`?

Can we first discuss what the right solution should be? And what's the upgrade story if we
still want to move from mesos::internal to mesos? At least, we want to make sure we have a
plausible way forward. Thoughts?


- Jie


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


On Feb. 18, 2015, 10:27 p.m., Kapil Arya wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31162/
> -----------------------------------------------------------
> 
> (Updated Feb. 18, 2015, 10:27 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Jie Yu, Niklas Nielsen, Till Toenshoff, and Vinod
Kone.
> 
> 
> Bugs: MESOS-2371
>     https://issues.apache.org/jira/browse/MESOS-2371
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This changeset puts the internal protos back into mesos::internal namespace.
> This is required for upgrade compatibility.  Without it, a newer Master would
> reject all messages from an older Slave and vice-versa.
> 
> https://reviews.apache.org/r/30300/
> 
> 
> Diffs
> -----
> 
>   include/mesos/authentication/authentication.hpp 699aa886286bc7d9c05592e71232ab1c1084871f

>   include/mesos/authentication/authentication.proto 38a6f781d6a0b4618a14e1681420564d78b840a8

>   include/mesos/module/module.hpp e83be2822b7c0e7935ab1c8af36e5cb9b5180f20 
>   include/mesos/module/module.proto 821fc0e72ece7c497595859fc5efc1c64ea49b9b 
>   include/mesos/type_utils.hpp cdf5864389a72002b538c263d70bcade2bdffa45 
>   src/common/parse.hpp 547b32041f39f0ff0c38179b66a32b2239134abc 
>   src/common/type_utils.cpp 12a36bbd7d7773b25dedf2d0d951c79e0b5141d6 
>   src/log/consensus.hpp ee9e9081ffe2a5f18433d9eff1a2e2cf17c81863 
>   src/log/leveldb.hpp 8f5df5bdfb08de02e4129494fba188f97c3b8010 
>   src/log/leveldb.cpp 1d679425c6df3498be67d048bc05d02ffe03a636 
>   src/log/replica.hpp b2602083564447cf75827257a2548c1b5aeb49e2 
>   src/log/storage.hpp 5e81f4e9774ad239145019320cf991fcf51a9da5 
>   src/master/registry.hpp d3cbe564dc8e14f3dd301e5bc06ed60d82856eee 
>   src/master/registry.proto 29a309763bca9db76c443d7c039ca152a2afbc5b 
>   src/messages/log.proto 12b3572e0fe0e30c5eff20c7af27eb3d7cfe8f4b 
>   src/messages/messages.hpp 25769b797ec9bd1a1c348c467616aef8407c45d6 
>   src/messages/messages.proto 58484ae45071a80afd2b11803dd66a88f88ad9ed 
>   src/messages/state.proto 7fc4883b9fe590afbbb0261988c929c3b1f8f676 
>   src/state/storage.hpp f5cd607b47d44f72d72cbd0c5adb7368410b83d8 
>   src/tests/rate_limiting_tests.cpp 784ac76ee81918466de44196f5adc2b88adfee96 
>   src/tests/state_tests.cpp b30812322dd8ae507e1d45912ab559940ac5ee73 
> 
> Diff: https://reviews.apache.org/r/31162/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> Also tested with master running HEAD with this patch, and slave running 0.21.0 with src/test-framework.
> 
> The tests succeed when src/test-framework is running from the same version as the slave.
 However, if I run src/test-framework from the master version, it doesn't succeed and errors
out with unexpected TASK_LOST.  Not sure if this is the desired behavior.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>


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