(The JIRA:) https://issues.apache.org/jira/browse/MESOS-2272
On Mon, Jan 26, 2015 at 10:50 PM, Kapil Arya <ka...@mesosphere.io> wrote: > PS: I have created a Jira and have published the following RRs: > 1. https://reviews.apache.org/r/30294/ > 2. https://reviews.apache.org/r/30295/ > 3. https://reviews.apache.org/r/30300/ > > > On Tue, Jan 27, 2015 at 1:50 AM, Kapil Arya <ka...@mesosphere.io> wrote: > > > Hi Jie, > > > > Thanks for the comments. I have tried to answer them inline. Please let > us > > know if something isn't clear. > > > > Kapil > > > > On Tue, Jan 27, 2015 at 1:25 AM, Jie Yu <yujie....@gmail.com> wrote: > > > >> One benefit of having an internal namespace is that it tells the > >> framework/executor writer that those symbols/method/class are internal > to > >> Mesos core and should not be used. > >> > > > > We don't have any internal symbols/methods/classes in public headers, do > > we? This is assuming that a framework writer installs a mesos-dev > package > > or equivalent and doesn't deal with mesos source tree. > > > > > >> If we kill all the internal namespaces and move many headers like > >> isolator.hpp to include/mesos, how does the framework writer know that > >> he/she shouldn't use some of the headers because they are internal to > >> Mesos > >> and are subject to change? > >> > > > > I do agree on your general point about exposing more files to framework > > writers. However, shouldn't a framework writer be using the headers, etc. > > based on their requirements rather than grabbing anything and everything > > that is exposed as public headers? > > > > > >> > >> For modules, I am wondering can we separate Mesos public headers (in > >> include/mesos right now) from those headers that are only for building > >> modules (more like internal public headers). > >> > > > > It's a bit complicated. There are some files that correspond only to the > > slave (i.e. mesos/slave/state.hpp, mesos/slave/isolator.hpp) and > similarly, > > very soon we'll have master-specific files as well. Similarly, there are > > some shared files such as those authenticator/authenticatee, that are > > shared by both master and slave. In all these cases, these files aren't > > just about modules, instead, modules are only _one_ of the consumers of > > these files. > > > > Further, module-specific files currently reside in mesos/module/. As per > > the current file layout, files in mesos/module/ #include files from > mesos/. > > For example, mesos/module/isolator.hpp #includes > mesos/slave/isolator.hpp. > > > > Is there an alternate file layout suggestion that we should think about? > > > > > >> Thoughts? > >> > >> - Jie > >> > >> On Mon, Jan 26, 2015 at 12:35 PM, Kapil Arya <ka...@mesosphere.io> > wrote: > >> > >> > Hi All, > >> > > >> > TLDR: We currently use "mesos::internal" namespace for almost > everything > >> > inside src/. However, in most cases, it is directly enclosing another > >> > namespace. This makes the "internal" namespace redundant and we > should > >> > kill it. > >> > > >> > I learned from Ben Hindman that the original motivation for > introducing > >> an > >> > explicit internal namespace was to discourage people from exposing > >> internal > >> > symbols through "extern", etc. Since we don't seem to expose symbols > >> > through "extern" in our codebase, I think it's safe to kill this > >> namespace. > >> > > >> > Here is a list of files that define classes directly in the > >> mesos::internal > >> > namespace (i.e. without enclosing a separate namespace) [1]: > >> > > >> > authorizer/authorizer.hpp > >> > common/http.hpp > >> > common/attributes.hpp > >> > common/lock.hpp > >> > files/files.hpp > >> > hook/manager.hpp > >> > master/contender.hpp > >> > master/detector.hpp > >> > usage/usage.hpp > >> > watcher/whitelist_watcher.hpp > >> > > >> > messages/messages.pb.h > >> > > >> > Of the above list, things like hook/manager.hpp and > >> > master/{contender,detector}.hpp can be moved into their own > >> namespaces. I > >> > am sure, we can come up with a strategy for the rest as well. > >> > > >> > One possible concern here might be messages.proto and the effects on > >> > upgrades, etc., if we change the namespace/package for these > >> protobufs. If > >> > this turns out to be a real concern, we can possibly keep the internal > >> > namespace for messages.proto. > >> > > >> > If we kill the "internal" namespace altogether, it would make it much > >> > easier to expose some headers as public headers for modularization, > >> etc.. > >> > This will also help us get rid of "namespace internal" from some of > the > >> > public headers that we already have. > >> > > >> > The motivation for killing the "internal" namespace comes from a > >> > patch-set[2] that tries to expose slave/containerizer/isolator.hpp as > >> > include/mesos/slave/isolator.hpp. We wanted to keep Isolator inside > the > >> > "mesos::slave" namespace instead of putting it directly in the > "mesos" > >> > namespace. This change causes a lot of conflicts due to > "mesos::slave" > >> and > >> > "mesos::internal::slave" and we had to resort to using fully qualified > >> > names for a large number of definition in src/slave, src/tests, etc. > >> > > >> > Best, > >> > Kapil > >> > > >> > [1] List obtained by running the following command and then filtering > >> out > >> > the instances where "internal" was enclosed in something other than > >> > "mesos": > >> > grep -nr "} // namespace internal" * -B1 |grep -v > >> > "namespace\|\-\-\|\.cpp"|cut -d'-' -f1 > >> > > >> > [2] https://reviews.apache.org/r/30238/ > >> > https://reviews.apache.org/r/29602/ > >> > https://reviews.apache.org/r/29603/ > >> > > >> > > > > >
