Re: Kill the internal namespace
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
Re: Kill the internal namespace
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/
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
Re: Kill the internal namespace
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
Re: Kill the internal namespace
(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
Re: Kill the internal namespace
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
Re: Kill the internal namespace
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/
Re: Kill the internal namespace
I should have been clearer. I didn't mean to kill internal from stout,
only from Mesos proper (src/ and include/mesos/).
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/
Re: Kill the internal namespace
+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.*
Re: Kill the internal namespace
SGTM. I would suggest to first address the non-proto files before changing
the proto files.
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/
Kill the internal namespace
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/
Re: Kill the internal namespace
SGTM - but it would probably be a good idea to do some preliminary testing
of the upgrade path it would result in.
Kapil, have you tried to run different mesos processes with and w/o the
internal namespace?
Niklas
On 26 January 2015 at 12:47, Vinod Kone vinodk...@gmail.com wrote:
SGTM. I would suggest to first address the non-proto files before changing
the proto files.
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/
Re: Kill the internal namespace
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/
Re: Kill the internal namespace
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/
Re: Kill the internal namespace
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/
Re: Kill the internal namespace
I want to note that if there was any change in behavior around this change that would likely indicate a serious compiler bug. Namespaces are something that is purely programmer visible in C++, the symbol names will change slightly for things that were internal, but nothing else. Things need to be recompiled to get the new ABI, but none of the network packets, etc. should change.
Re: Kill the internal namespace
On Mon, Jan 26, 2015 at 2:09 PM, Niklas Nielsen nik...@mesosphere.io
wrote:
SGTM - but it would probably be a good idea to do some preliminary testing
of the upgrade path it would result in.
Kapil, have you tried to run different mesos processes with and w/o the
internal namespace?
I haven't tried it yet, but will do soon. Is there a particular set of
tests that you would like?
Kapil
Niklas
On 26 January 2015 at 12:47, Vinod Kone vinodk...@gmail.com wrote:
SGTM. I would suggest to first address the non-proto files before
changing
the proto files.
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/
