Thanks Ben! These are really important question. On a high level, I agree to all you concerns. However, I do think that the current patch set doesn't change anything drastically with regard to upgrades.
On Thu, Jan 15, 2015 at 11:38 AM, Benjamin Mahler <[email protected] > wrote: > As we push more things towards modularization, what are the implications of > breaking these APIs? How are folks running modules going to do upgrades? > > When we break the APIs, are we willing to accept that the upgrade of the > component (slave or master) and its modules, must occur in lock-step? > I think nothing has changed on that front since the first push to create the module system. By default, modules still come with the conservative approach of strict version compatibility checks and fail to load if there is a version mismatch. (The design allows the module writer to relax some of the checks if needed, but let's stick with the default case for this discussion). If so, great! If not, we're pouring some cement on our existing technical > debt (given we're not doing any refactoring before pushing these APIs out > to public scope). It's true that we are exposing previously unexposed files, but that doesn't change much in terms of how the modules are handled Today. A module writer currently depends upon these files anyways and thus these files are, in a sense, already in the public scope. Having said that, I do agree with your suggestion of refactoring of these APIs. In the current patch set, the way it is, the only refactoring we might need is for the Isolator module. The authentication modules are fairly self contained and have a very small API which I believe doesn't require any refactoring. Till can comment more on that. Kapil > On Thu, Jan 15, 2015 at 11:10 AM, Kapil Arya <[email protected]> wrote: > > > It would be really helpful to us if we can get some feedback by the end > of > > the week so that we can prepare to commit early next week. > > > > Some of the RRs have already received some reviews and having more of > that > > will be helpful too. > > > > Kapil > > > > On Mon, Jan 12, 2015 at 2:57 PM, Kapil Arya <[email protected]> wrote: > > > > > Hi All, > > > > > > We have been working to allow Mesos modules to be built outside the > Mesos > > > source tree (or without the need to build Mesos) and wanted to get some > > > community feedback. The issue is tracked here: > > > https://issues.apache.org/jira/browse/MESOS-2096. > > > > > > Best, > > > Kapil > > > > > > > > > Objective: Build Mesos modules by using just the publicly exposed > > > headers. This necessarily means that a third party contributor would > > only > > > need to install the mesos-dev package in order to build a module. > > > > > > Out of scope: Ideally, we want to ensure version compatibility of > > various > > > third-party components (e.g., protobufs and glog) used by Mesos and > > > modules. One possibility is to create a utility like mesos-config > > similar > > > in spirit to python-config that emits configuration options that were > > used > > > to build Mesos. The configure script for a third-party module can then > > > decide upon the compatibility. > > > > > > Here are the required changes for our objective: > > > > > > A) Expose some internal protobufs (by putting them in mesos.proto). > > > Examples of such protobufs include Modules, and any other protobufs > > needed > > > by the files being exposed (see B). Along with exposing said > protobufs, > > we > > > need to expose the corresponding operators definitions that are not > > > generated by the protobuf compiler (i.e. definitions in > > > common/type_utils.hpp). > > > > > > B) Expose Isolator headers (slave/containerizer/isolator.hpp and > > > slave/state.hpp). Use mesos::slave for isolator.hpp and > > mesos::slave::state > > > for state.hpp. Isolator depends on slave/state.hpp and so we need to > > > expose both. This also requires us to disentangle these two files from > > the > > > rest of the code. > > > > > > C) Expose authentication headers. This is similar in spirit to B. > > > > > > > > > Next is a list of RRs that address A, B, and C. > > > > > > A. Expose internal protobufs: > > > 1. Moved Modules protobuf to mesos namespace by moving it into > > > mesos.proto > > > (https://reviews.apache.org/r/29598/) > > > > > > 2. Moved Task and StatusUpdate from messages.proto to mesos.proto. > > > (These protobufs are used by the Isolator header) > > > (https://reviews.apache.org/r/29600/) > > > > > > 3. Moved authentication messages to mesos.proto. > > > (https://reviews.apache.org/r/29605) > > > > > > 4. Moved operators for internal protobufs to messages/messages.hpp. > > > This allows type_utils.hpp to only have operators corresponding to > public > > > protobufs defined in mesos.proto. > > > (https://reviews.apache.org/r/2978) > > > > > > 5. Exposed type_utils.hpp as include/mesos/type_utils.hpp. > > > (https://reviews.apache.org/r/29781) > > > > > > B. Expose Isolator headers: > > > > > > 6. Move mesos::internal::slave::state to mesos::slave::state. > > > (https://reviews.apache.org/r/29782) > > > > > > 7. Moved mesos::internal::slave::Isolator to mesos::slave > namespace. > > > (https://reviews.apache.org/r/29602) > > > > > > 8. Exposed slave/containerizer/isolator.hpp and slave/state.hpp. > > > (https://reviews.apache.org/r/29603) > > > > > > 9. Exposed module/isolator.hpp for out-of-tree module building. > > > Review: https://reviews.apache.org/r/29604 > > > > > > C. Expose authentication headers > > > > > > 10. Exposed authentication headers. > > > (https://reviews.apache.org/r/29606) > > > > > >
