> On Nov. 4, 2013, 8:24 p.m., Vinod Kone wrote: > > include/mesos/mesos.proto, lines 179-180 > > <https://reviews.apache.org/r/14847/diff/2/?file=370571#file370571line179> > > > > Sorry didn't review this earlier but there are couple of issues with > > this. > > > > SlaveInfo is checkpointed, so changing the proto doesn't necessarily > > guarantee that what's already on disk will change. Infact the slave > > constructs its SlaveInfo from the checkpointed SlaveInfo. In this > > particular case it is probably safe because it will just ignore the > > webui_hostname and webui_port fields. > > > > Removing a required field is almost always a bad idea? In this case an > > old executor/master cannot work with a new slave that is using the updated > > SlaveInfo because the required field webui_hostname will be missing.
I'm missing the incompatibility issues. The 'webui_hostname' and 'webui_port' fields have been deprecated for a while now and except for in a few tests and in Slave::initialize they were not being used (including in the master). A quick inspection shows that the last version to use webui_hostname was 0.10, which for a volatile project like Mesos is sufficient deprecation time (we've deprecated other things after just one release!). Am I missing something else? - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14847/#review28134 ----------------------------------------------------------- On Oct. 24, 2013, 3:26 a.m., Niklas Nielsen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/14847/ > ----------------------------------------------------------- > > (Updated Oct. 24, 2013, 3:26 a.m.) > > > Review request for mesos and Benjamin Hindman. > > > Repository: mesos-git > > > Description > ------- > > A slave can be started with --hostname which explicitly sets hostname instead > of usual system hostname. > This is necessary in situations where system hostname resolves to internal > names which cannot be accessed from the web ui. > > > Diffs > ----- > > include/mesos/mesos.proto fe1d82b > src/common/type_utils.hpp c48411f > src/slave/flags.hpp db777e3 > src/slave/slave.cpp debb2f4 > src/tests/state_tests.cpp f39dee5 > > Diff: https://reviews.apache.org/r/14847/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Niklas Nielsen > >
