On Tue, Jan 27, 2015 at 10:28 AM, Jie Yu <yujie....@gmail.com> wrote:
> I think Mesos is kind of different than other C/C++ projects like apache
> httpd, llvm, etc. For httpd, it's clear that the public headers are for
> module writers. For llvm, the public headers are for plugin writers.
>
> For Mesos, we have two types of developers: framework writers and module
> writers. I think it'll be great if we can separate headers for these two
> types of writers in an obvious way.
>
In the near term, framework writers should be interacting with the pure
HTTP API at which point headers will only be for module writers, and
protobufs will be the only requirement for framework writers.
>
> What I am proposing is: we still keep the internal namespace in Mesos, but
> move those internal interfaces that needed by the module to
> include/mesos/internal. So the layout of the include/ is like the
> following:
>
> include/
> |-- mesos/ <-- public headers for framework/executor writers
> |-- scheduler.hpp
> |-- executor.hpp
> ...
> |-- internal/ <-- headers for module writers
> |--- slave/
> |--- master/
>
> Thoughts?
>
> - Jie
>
>
> On Tue, Jan 27, 2015 at 10:04 AM, Dominic Hamon <dha...@twopensource.com>
> wrote:
>
> > +1
> >
> > if people write comments that say that, i'll be the first to recommend a
> > redesign of what they're writing :)
> >
> >
> >
> > On Tue, Jan 27, 2015 at 8:47 AM, Benjamin Hindman <
> b...@eecs.berkeley.edu>
> > wrote:
> >
> > > I think the point that Jie is making is that we have sometimes wanted
> to
> > > have "internal" implementation details exposed in public headers. This
> is
> > > common in Stout because everything has to be in headers.
> > >
> > > With that in mind, our goal for Mesos should be to have _zero_ internal
> > > implementation details in public headers. If people find themselves
> > writing
> > > comments that say "don't use these global variables or functions
> because
> > > they're internal to the implementation" then I'll be the first to
> > recommend
> > > that we reintroduce the 'internal' namespace.
> > >
> > > As for Stout I think we should keep the 'internal' namespace.
> > >
> > > On Mon, Jan 26, 2015 at 10:25 PM, 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.
> > > >
> > > > 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?
> > > >
> > > > 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).
> > > >
> > > > 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/
> > > > >
> > > >
> > >
> >
> >
> >
> > --
> > Dominic Hamon | @mrdo | Twitter
> > *There are no bad ideas; only good ideas that go horribly wrong.*
> >
>
--
Dominic Hamon | @mrdo | Twitter
*There are no bad ideas; only good ideas that go horribly wrong.*
