You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Greg Mann <gr...@mesosphere.io> on 2016/04/15 08:51:42 UTC
Re: Review Request 46254: Added a realm parameter to
`process::initialize` (libprocess).
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46254/
-----------------------------------------------------------
(Updated April 15, 2016, 6:51 a.m.)
Review request for mesos, Adam B and Alexander Rojas.
Summary (updated)
-----------------
Added a realm parameter to `process::initialize` (libprocess).
Bugs: MESOS-4902
https://issues.apache.org/jira/browse/MESOS-4902
Repository: mesos
Description (updated)
-------
In order to enable authentication on libprocess-level
HTTP endpoints, this patch adds a parameter to
process::initialize which allows the authentication
realm of such endpoints to be set when libprocess is
initialized.
Diffs (updated)
-----
3rdparty/libprocess/include/process/gtest.hpp 3e0887257e1484813b3547170a5a1b9bb89de0d2
3rdparty/libprocess/include/process/process.hpp 77e96957ca556cf9a7e2bf427d6762572fe48c51
3rdparty/libprocess/src/process.cpp 5e9dcfdc52f3a8223bc43af149b8e1f5dbdf5b0a
3rdparty/libprocess/src/tests/main.cpp 78858a2b84a439d8f8a60ec8bcb6ac3a308087a6
Diff: https://reviews.apache.org/r/46254/diff/
Testing
-------
`sudo make check` on OSX.
Thanks,
Greg Mann
Re: Review Request 46254: Added a realm parameter to
`process::initialize` (libprocess).
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46254/#review129310
-----------------------------------------------------------
Ship it!
Ship It!
- Alexander Rojas
On April 15, 2016, 7:49 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46254/
> -----------------------------------------------------------
>
> (Updated April 15, 2016, 7:49 p.m.)
>
>
> Review request for mesos, Adam B and Alexander Rojas.
>
>
> Bugs: MESOS-4902
> https://issues.apache.org/jira/browse/MESOS-4902
>
>
> Repository: mesos
>
>
> Description
> -------
>
> In order to enable authentication on libprocess-level
> HTTP endpoints, this patch adds a parameter to
> process::initialize which allows the authentication
> realm of such endpoints to be set when libprocess is
> initialized.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/gtest.hpp 3e0887257e1484813b3547170a5a1b9bb89de0d2
> 3rdparty/libprocess/include/process/process.hpp 77e96957ca556cf9a7e2bf427d6762572fe48c51
> 3rdparty/libprocess/src/process.cpp 5e9dcfdc52f3a8223bc43af149b8e1f5dbdf5b0a
> 3rdparty/libprocess/src/tests/main.cpp 78858a2b84a439d8f8a60ec8bcb6ac3a308087a6
>
> Diff: https://reviews.apache.org/r/46254/diff/
>
>
> Testing
> -------
>
> `sudo make check` on OSX.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 46254: Added a realm parameter to
`process::initialize` (libprocess).
Posted by Kapil Arya <ka...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46254/#review130201
-----------------------------------------------------------
Ship it!
Ship It!
- Kapil Arya
On April 21, 2016, 4:21 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46254/
> -----------------------------------------------------------
>
> (Updated April 21, 2016, 4:21 p.m.)
>
>
> Review request for mesos, Adam B and Alexander Rojas.
>
>
> Bugs: MESOS-4951
> https://issues.apache.org/jira/browse/MESOS-4951
>
>
> Repository: mesos
>
>
> Description
> -------
>
> In order to enable authentication on libprocess-level
> HTTP endpoints, this patch adds a parameter to
> process::initialize which allows the authentication
> realm of such endpoints to be set when libprocess is
> initialized.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/gtest.hpp 3e0887257e1484813b3547170a5a1b9bb89de0d2
> 3rdparty/libprocess/include/process/process.hpp 77e96957ca556cf9a7e2bf427d6762572fe48c51
> 3rdparty/libprocess/src/process.cpp 5e9dcfdc52f3a8223bc43af149b8e1f5dbdf5b0a
> 3rdparty/libprocess/src/tests/main.cpp 78858a2b84a439d8f8a60ec8bcb6ac3a308087a6
>
> Diff: https://reviews.apache.org/r/46254/diff/
>
>
> Testing
> -------
>
> `sudo make check` on OSX.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 46254: Added a realm parameter to
`process::initialize` (libprocess).
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46254/
-----------------------------------------------------------
(Updated April 21, 2016, 8:21 p.m.)
Review request for mesos, Adam B and Alexander Rojas.
Bugs: MESOS-4951
https://issues.apache.org/jira/browse/MESOS-4951
Repository: mesos
Description
-------
In order to enable authentication on libprocess-level
HTTP endpoints, this patch adds a parameter to
process::initialize which allows the authentication
realm of such endpoints to be set when libprocess is
initialized.
Diffs
-----
3rdparty/libprocess/include/process/gtest.hpp 3e0887257e1484813b3547170a5a1b9bb89de0d2
3rdparty/libprocess/include/process/process.hpp 77e96957ca556cf9a7e2bf427d6762572fe48c51
3rdparty/libprocess/src/process.cpp 5e9dcfdc52f3a8223bc43af149b8e1f5dbdf5b0a
3rdparty/libprocess/src/tests/main.cpp 78858a2b84a439d8f8a60ec8bcb6ac3a308087a6
Diff: https://reviews.apache.org/r/46254/diff/
Testing
-------
`sudo make check` on OSX.
Thanks,
Greg Mann
Re: Review Request 46254: Added a realm parameter to
`process::initialize` (libprocess).
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46254/
-----------------------------------------------------------
(Updated April 21, 2016, 4:23 p.m.)
Review request for mesos, Adam B and Alexander Rojas.
Bugs: MESOS-4951
https://issues.apache.org/jira/browse/MESOS-4951
Repository: mesos
Description
-------
In order to enable authentication on libprocess-level
HTTP endpoints, this patch adds a parameter to
process::initialize which allows the authentication
realm of such endpoints to be set when libprocess is
initialized.
Diffs
-----
3rdparty/libprocess/include/process/gtest.hpp 3e0887257e1484813b3547170a5a1b9bb89de0d2
3rdparty/libprocess/include/process/process.hpp 77e96957ca556cf9a7e2bf427d6762572fe48c51
3rdparty/libprocess/src/process.cpp 5e9dcfdc52f3a8223bc43af149b8e1f5dbdf5b0a
3rdparty/libprocess/src/tests/main.cpp 78858a2b84a439d8f8a60ec8bcb6ac3a308087a6
Diff: https://reviews.apache.org/r/46254/diff/
Testing
-------
`sudo make check` on OSX.
Thanks,
Greg Mann
Re: Review Request 46254: Added a realm parameter to
`process::initialize` (libprocess).
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46254/
-----------------------------------------------------------
(Updated April 19, 2016, 4:35 p.m.)
Review request for mesos, Adam B and Alexander Rojas.
Bugs: MESOS-4951
https://issues.apache.org/jira/browse/MESOS-4951
Repository: mesos
Description
-------
In order to enable authentication on libprocess-level
HTTP endpoints, this patch adds a parameter to
process::initialize which allows the authentication
realm of such endpoints to be set when libprocess is
initialized.
Diffs
-----
3rdparty/libprocess/include/process/gtest.hpp 3e0887257e1484813b3547170a5a1b9bb89de0d2
3rdparty/libprocess/include/process/process.hpp 77e96957ca556cf9a7e2bf427d6762572fe48c51
3rdparty/libprocess/src/process.cpp 5e9dcfdc52f3a8223bc43af149b8e1f5dbdf5b0a
3rdparty/libprocess/src/tests/main.cpp 78858a2b84a439d8f8a60ec8bcb6ac3a308087a6
Diff: https://reviews.apache.org/r/46254/diff/
Testing
-------
`sudo make check` on OSX.
Thanks,
Greg Mann
Re: Review Request 46254: Added a realm parameter to
`process::initialize` (libprocess).
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46254/
-----------------------------------------------------------
(Updated April 15, 2016, 5:49 p.m.)
Review request for mesos, Adam B and Alexander Rojas.
Bugs: MESOS-4902
https://issues.apache.org/jira/browse/MESOS-4902
Repository: mesos
Description
-------
In order to enable authentication on libprocess-level
HTTP endpoints, this patch adds a parameter to
process::initialize which allows the authentication
realm of such endpoints to be set when libprocess is
initialized.
Diffs (updated)
-----
3rdparty/libprocess/include/process/gtest.hpp 3e0887257e1484813b3547170a5a1b9bb89de0d2
3rdparty/libprocess/include/process/process.hpp 77e96957ca556cf9a7e2bf427d6762572fe48c51
3rdparty/libprocess/src/process.cpp 5e9dcfdc52f3a8223bc43af149b8e1f5dbdf5b0a
3rdparty/libprocess/src/tests/main.cpp 78858a2b84a439d8f8a60ec8bcb6ac3a308087a6
Diff: https://reviews.apache.org/r/46254/diff/
Testing
-------
`sudo make check` on OSX.
Thanks,
Greg Mann
Re: Review Request 46254: Added a realm parameter to
`process::initialize` (libprocess).
Posted by Alexander Rojas <al...@mesosphere.io>.
> On April 15, 2016, 12:35 p.m., Alexander Rojas wrote:
> > 3rdparty/libprocess/include/process/gtest.hpp, line 31
> > <https://reviews.apache.org/r/46254/diff/2/?file=1346267#file1346267line31>
> >
> > Something tells me there should be a comment here, but I don't know exactly what its contents should be.
I originaly thought this was in a cpp, but there are multiple problems I see:
1. We avoid ha static global strings since the have initialization uncertainty, also if something goes wrong it can be destroy before you can use it.
2. The way to go is `constexpr char DEFAULT_HTTP_AUTHENTICATION_REALM[] = "libprocess-realm"`
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46254/#review129091
-----------------------------------------------------------
On April 15, 2016, 8:51 a.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46254/
> -----------------------------------------------------------
>
> (Updated April 15, 2016, 8:51 a.m.)
>
>
> Review request for mesos, Adam B and Alexander Rojas.
>
>
> Bugs: MESOS-4902
> https://issues.apache.org/jira/browse/MESOS-4902
>
>
> Repository: mesos
>
>
> Description
> -------
>
> In order to enable authentication on libprocess-level
> HTTP endpoints, this patch adds a parameter to
> process::initialize which allows the authentication
> realm of such endpoints to be set when libprocess is
> initialized.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/gtest.hpp 3e0887257e1484813b3547170a5a1b9bb89de0d2
> 3rdparty/libprocess/include/process/process.hpp 77e96957ca556cf9a7e2bf427d6762572fe48c51
> 3rdparty/libprocess/src/process.cpp 5e9dcfdc52f3a8223bc43af149b8e1f5dbdf5b0a
> 3rdparty/libprocess/src/tests/main.cpp 78858a2b84a439d8f8a60ec8bcb6ac3a308087a6
>
> Diff: https://reviews.apache.org/r/46254/diff/
>
>
> Testing
> -------
>
> `sudo make check` on OSX.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 46254: Added a realm parameter to
`process::initialize` (libprocess).
Posted by Greg Mann <gr...@mesosphere.io>.
> On April 15, 2016, 10:35 a.m., Alexander Rojas wrote:
> > 3rdparty/libprocess/include/process/gtest.hpp, line 31
> > <https://reviews.apache.org/r/46254/diff/2/?file=1346267#file1346267line31>
> >
> > Something tells me there should be a comment here, but I don't know exactly what its contents should be.
>
> Alexander Rojas wrote:
> I originaly thought this was in a cpp, but there are multiple problems I see:
>
> 1. We avoid ha static global strings since the have initialization uncertainty, also if something goes wrong it can be destroy before you can use it.
> 2. The way to go is `constexpr char DEFAULT_HTTP_AUTHENTICATION_REALM[] = "libprocess-realm"`
Thanks arojas! I changed this to `constexpr char`, which I believe fixes the uncertain initialization/lifetime issue, but you said "multiple problems" so let me know what you think :-)
- Greg
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46254/#review129091
-----------------------------------------------------------
On April 15, 2016, 5:49 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46254/
> -----------------------------------------------------------
>
> (Updated April 15, 2016, 5:49 p.m.)
>
>
> Review request for mesos, Adam B and Alexander Rojas.
>
>
> Bugs: MESOS-4902
> https://issues.apache.org/jira/browse/MESOS-4902
>
>
> Repository: mesos
>
>
> Description
> -------
>
> In order to enable authentication on libprocess-level
> HTTP endpoints, this patch adds a parameter to
> process::initialize which allows the authentication
> realm of such endpoints to be set when libprocess is
> initialized.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/gtest.hpp 3e0887257e1484813b3547170a5a1b9bb89de0d2
> 3rdparty/libprocess/include/process/process.hpp 77e96957ca556cf9a7e2bf427d6762572fe48c51
> 3rdparty/libprocess/src/process.cpp 5e9dcfdc52f3a8223bc43af149b8e1f5dbdf5b0a
> 3rdparty/libprocess/src/tests/main.cpp 78858a2b84a439d8f8a60ec8bcb6ac3a308087a6
>
> Diff: https://reviews.apache.org/r/46254/diff/
>
>
> Testing
> -------
>
> `sudo make check` on OSX.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 46254: Added a realm parameter to
`process::initialize` (libprocess).
Posted by Alexander Rojas <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/46254/#review129091
-----------------------------------------------------------
3rdparty/libprocess/include/process/gtest.hpp (line 31)
<https://reviews.apache.org/r/46254/#comment192561>
Something tells me there should be a comment here, but I don't know exactly what its contents should be.
- Alexander Rojas
On April 15, 2016, 8:51 a.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46254/
> -----------------------------------------------------------
>
> (Updated April 15, 2016, 8:51 a.m.)
>
>
> Review request for mesos, Adam B and Alexander Rojas.
>
>
> Bugs: MESOS-4902
> https://issues.apache.org/jira/browse/MESOS-4902
>
>
> Repository: mesos
>
>
> Description
> -------
>
> In order to enable authentication on libprocess-level
> HTTP endpoints, this patch adds a parameter to
> process::initialize which allows the authentication
> realm of such endpoints to be set when libprocess is
> initialized.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/gtest.hpp 3e0887257e1484813b3547170a5a1b9bb89de0d2
> 3rdparty/libprocess/include/process/process.hpp 77e96957ca556cf9a7e2bf427d6762572fe48c51
> 3rdparty/libprocess/src/process.cpp 5e9dcfdc52f3a8223bc43af149b8e1f5dbdf5b0a
> 3rdparty/libprocess/src/tests/main.cpp 78858a2b84a439d8f8a60ec8bcb6ac3a308087a6
>
> Diff: https://reviews.apache.org/r/46254/diff/
>
>
> Testing
> -------
>
> `sudo make check` on OSX.
>
>
> Thanks,
>
> Greg Mann
>
>