> On June 20, 2016, 8:06 a.m., Alexander Rukletsov wrote: > > To avoid confusion, we should actually change the namespace allocator code > > lives is as well. I've once started that effort > > (https://reviews.apache.org/r/29930/) but decided to discard because > > allocator is the master-only component. > > Zhitao Li wrote: > Alexander, so what's your recommendation here? Given that there is only > one public visible message `InverseOfferStatus` in this protobuf at the > moment, I'd like to have a quick fix to make sure its namespace does not > change after v1.0 and catch the release deadline. > > If "allocator is the master-only component" is considered a good reason > to keep these code in master namespace, then the question is how to handle > maintenance which could have circular dependency to master components: maybe > we should just move this message to `mesos` namespace as other high level > messages like `Offer` or `TaskStatus`? > > haosdent huang wrote: > >make sure its namespace does not change after v1.0 and catch the release > deadline. > > +1 for this. I think all we don't want to break the V1 protobuf files > after 1.0 release > > I am still curious why could not put allocator.proto and master.proto > under the same folder. > If it is forbidden and we want to keep allocator.proto under master > namespace, I think we could > move `message InverseOfferStatus` from allocator.proto to master.proto > and then remove allocator.proto.
@haosdent, the goal is here is to both make the protobuf structures consistent between packages, and avoid possible circular dependency for various languages. For golang, including a protobuf also generates an import to the package included protobuf file is in. Therefore, `master` package imports `maintenance`, and `maintenance` imports `master` (though this allocator.proto file) and causes a circular dependency. After some thoughts, the only `InverseOfferStatus` message in allocator.proto definitely should not belong to `master` package, because the latter is pretty much used soly for the new operator HTTP API. Given that this message spans all "allocator", "Mesos master" and "maintenance" logic concepts, it's common enough to be promoted to `include/mesos/v1/mesos.proto` and be put next to `message InverseOffer`. And if we move this one, we can simply drop this allocator.proto file because it'll be empty. (Alternatively, if we don't think it's common enough, moving it to `include/mesos/v1/maintenance/maintenance.proto` is acceptable to me). Note that I'm only talking about the messages namespaced in v1. I agree we should make non versioned messages consistent here, but that could be done in a separate patch. - Zhitao ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48902/#review138546 ----------------------------------------------------------- On June 18, 2016, 6:36 a.m., Zhitao Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48902/ > ----------------------------------------------------------- > > (Updated June 18, 2016, 6:36 a.m.) > > > Review request for mesos, Anand Mazumdar and Vinod Kone. > > > Bugs: MESOS-5642 > https://issues.apache.org/jira/browse/MESOS-5642 > > > Repository: mesos > > > Description > ------- > > Move v1/master/allocator.proto to its own package. > > > Diffs > ----- > > include/mesos/v1/maintenance/maintenance.proto > 053d72d7d9fa9ec8f572136020546fa4f955ab09 > include/mesos/v1/master/allocator.proto > cf416d173bc487aecbbec75c9ee391a54bf5327b > src/CMakeLists.txt d66186217c1319d4497640614ed4beee28602c38 > src/Makefile.am a4931560f1a5b3fbe41ea181477341d3ac459b58 > > Diff: https://reviews.apache.org/r/48902/diff/ > > > Testing > ------- > > run `make` on Mac. > > > Thanks, > > Zhitao Li > >
