mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Adam Bordelon <a...@mesosphere.io>
Subject Re: Kill the "internal" namespace
Date Tue, 27 Jan 2015 07:59:04 GMT
(The JIRA:)
https://issues.apache.org/jira/browse/MESOS-2272

On Mon, Jan 26, 2015 at 10:50 PM, Kapil Arya <kapil@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 <kapil@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.jay@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 <kapil@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/
> >> >
> >>
> >
> >
>

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