As Dominic pointed out, if the framework writers are not going to use the
headers, then it doesn't make much sense to create a hierarchy for
"internal" module-specific stuff at this point.
On Tue, Jan 27, 2015 at 2:12 PM, Kapil Arya <ka...@mesosphere.io> wrote:
>
>
> 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.
>>
>> 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/
>>
>
> I think, having "internal" in a public headers isn't a good idea. The
> module API is supposed to be fairly stable. This, in fact, will be the
> driving force to enable smooth upgrades even when there are some modules
> installed for master/slave.
>
>
>> 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.*
>> >
>>
>
>
