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 2017/03/23 16:21:02 UTC
Review Request 57882: Added SecretGenerator injection to the
MockSlave.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57882/
-----------------------------------------------------------
Review request for mesos, Anand Mazumdar and Vinod Kone.
Bugs: MESOS-6999
https://issues.apache.org/jira/browse/MESOS-6999
Repository: mesos
Description
-------
This patch adds an overload of `Slave::initialize` to the
mock slave for the purpose of injecting a secret generator.
Diffs
-----
src/tests/mock_slave.hpp 1acb961b642e1e0b4677db7b8fc6150d480eb751
src/tests/mock_slave.cpp 50c04bff9e6f0f202af4c07b4036e021d3833ebf
Diff: https://reviews.apache.org/r/57882/diff/1/
Testing
-------
Testing details can be found at the end of this chain.
Thanks,
Greg Mann
Re: Review Request 57882: Added SecretGenerator injection to the
MockSlave.
Posted by Greg Mann <gr...@mesosphere.io>.
> On March 24, 2017, 4:23 p.m., Anand Mazumdar wrote:
> > src/tests/mock_slave.hpp
> > Lines 206 (patched)
> > <https://reviews.apache.org/r/57882/diff/2/?file=1673068#file1673068line206>
> >
> > How about keeping this consistent now that we are using `Owned` on the agent? Use `Option<Owned<>>` instead.
> >
> > Modify the above comment accordingly.
Dropping these, since we switch to a raw pointer instead.
- Greg
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57882/#review170023
-----------------------------------------------------------
On March 24, 2017, 9:24 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57882/
> -----------------------------------------------------------
>
> (Updated March 24, 2017, 9:24 p.m.)
>
>
> Review request for mesos, Anand Mazumdar and Vinod Kone.
>
>
> Bugs: MESOS-6999
> https://issues.apache.org/jira/browse/MESOS-6999
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds an overload of `Slave::initialize` to the
> mock slave for the purpose of injecting a secret generator.
>
>
> Diffs
> -----
>
> src/tests/mock_slave.hpp 1acb961b642e1e0b4677db7b8fc6150d480eb751
> src/tests/mock_slave.cpp 50c04bff9e6f0f202af4c07b4036e021d3833ebf
>
>
> Diff: https://reviews.apache.org/r/57882/diff/3/
>
>
> Testing
> -------
>
> Testing details can be found at the end of this chain.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 57882: Added SecretGenerator injection to the
MockSlave.
Posted by Greg Mann <gr...@mesosphere.io>.
> On March 24, 2017, 4:23 p.m., Anand Mazumdar wrote:
> > src/tests/mock_slave.hpp
> > Lines 118 (patched)
> > <https://reviews.apache.org/r/57882/diff/2/?file=1673068#file1673068line118>
> >
> > `
> > virtual void initialize() override;
> > ` ?
The compiler complains about inconsistent use of override if we add this, so I'm leaving it out for now. We could do a batch update of all the mock agent methods later if we want. There is already a ticket for doing a sweep of the entire codebase to add `override`: MESOS-4871
- Greg
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57882/#review170023
-----------------------------------------------------------
On March 24, 2017, 9:24 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57882/
> -----------------------------------------------------------
>
> (Updated March 24, 2017, 9:24 p.m.)
>
>
> Review request for mesos, Anand Mazumdar and Vinod Kone.
>
>
> Bugs: MESOS-6999
> https://issues.apache.org/jira/browse/MESOS-6999
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds an overload of `Slave::initialize` to the
> mock slave for the purpose of injecting a secret generator.
>
>
> Diffs
> -----
>
> src/tests/mock_slave.hpp 1acb961b642e1e0b4677db7b8fc6150d480eb751
> src/tests/mock_slave.cpp 50c04bff9e6f0f202af4c07b4036e021d3833ebf
>
>
> Diff: https://reviews.apache.org/r/57882/diff/3/
>
>
> Testing
> -------
>
> Testing details can be found at the end of this chain.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 57882: Added SecretGenerator injection to the
MockSlave.
Posted by Anand Mazumdar <an...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57882/#review170023
-----------------------------------------------------------
src/tests/mock_slave.hpp
Lines 118 (patched)
<https://reviews.apache.org/r/57882/#comment242758>
`
virtual void initialize() override;
` ?
src/tests/mock_slave.hpp
Lines 206 (patched)
<https://reviews.apache.org/r/57882/#comment242753>
How about keeping this consistent now that we are using `Owned` on the agent? Use `Option<Owned<>>` instead.
Modify the above comment accordingly.
src/tests/mock_slave.cpp
Lines 156 (patched)
<https://reviews.apache.org/r/57882/#comment242756>
Use `swap()` instead once it's changed to `Owned`. Also, invoke `reset` on the current pointer after its swapped.
- Anand Mazumdar
On March 23, 2017, 8:44 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57882/
> -----------------------------------------------------------
>
> (Updated March 23, 2017, 8:44 p.m.)
>
>
> Review request for mesos, Anand Mazumdar and Vinod Kone.
>
>
> Bugs: MESOS-6999
> https://issues.apache.org/jira/browse/MESOS-6999
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds an overload of `Slave::initialize` to the
> mock slave for the purpose of injecting a secret generator.
>
>
> Diffs
> -----
>
> src/tests/mock_slave.hpp 1acb961b642e1e0b4677db7b8fc6150d480eb751
> src/tests/mock_slave.cpp 50c04bff9e6f0f202af4c07b4036e021d3833ebf
>
>
> Diff: https://reviews.apache.org/r/57882/diff/2/
>
>
> Testing
> -------
>
> Testing details can be found at the end of this chain.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 57882: Added SecretGenerator injection to the
MockSlave.
Posted by Anand Mazumdar <an...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57882/#review170092
-----------------------------------------------------------
Fix it, then Ship it!
src/tests/mock_slave.hpp
Lines 204 (patched)
<https://reviews.apache.org/r/57882/#comment242880>
New line before?
Also how about:
```cpp
// Set to the base class `secretGenerator` in `initialize()`. After `initialize()` has executed,
// this will be `None()`.
```
- Anand Mazumdar
On March 24, 2017, 9:24 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57882/
> -----------------------------------------------------------
>
> (Updated March 24, 2017, 9:24 p.m.)
>
>
> Review request for mesos, Anand Mazumdar and Vinod Kone.
>
>
> Bugs: MESOS-6999
> https://issues.apache.org/jira/browse/MESOS-6999
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds an overload of `Slave::initialize` to the
> mock slave for the purpose of injecting a secret generator.
>
>
> Diffs
> -----
>
> src/tests/mock_slave.hpp 1acb961b642e1e0b4677db7b8fc6150d480eb751
> src/tests/mock_slave.cpp 50c04bff9e6f0f202af4c07b4036e021d3833ebf
>
>
> Diff: https://reviews.apache.org/r/57882/diff/3/
>
>
> Testing
> -------
>
> Testing details can be found at the end of this chain.
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 57882: Added SecretGenerator injection to the
MockSlave.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57882/
-----------------------------------------------------------
(Updated March 25, 2017, 6:27 p.m.)
Review request for mesos, Anand Mazumdar and Vinod Kone.
Bugs: MESOS-6999
https://issues.apache.org/jira/browse/MESOS-6999
Repository: mesos
Description
-------
This patch adds an overload of `Slave::initialize` to the
mock slave for the purpose of injecting a secret generator.
Diffs (updated)
-----
src/tests/mock_slave.hpp 1acb961b642e1e0b4677db7b8fc6150d480eb751
src/tests/mock_slave.cpp 50c04bff9e6f0f202af4c07b4036e021d3833ebf
Diff: https://reviews.apache.org/r/57882/diff/4/
Changes: https://reviews.apache.org/r/57882/diff/3-4/
Testing
-------
Testing details can be found at the end of this chain.
Thanks,
Greg Mann
Re: Review Request 57882: Added SecretGenerator injection to the
MockSlave.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57882/
-----------------------------------------------------------
(Updated March 24, 2017, 9:24 p.m.)
Review request for mesos, Anand Mazumdar and Vinod Kone.
Changes
-------
Changed the `mockSecretGenerator` from `Owned` to raw pointer.
Bugs: MESOS-6999
https://issues.apache.org/jira/browse/MESOS-6999
Repository: mesos
Description
-------
This patch adds an overload of `Slave::initialize` to the
mock slave for the purpose of injecting a secret generator.
Diffs (updated)
-----
src/tests/mock_slave.hpp 1acb961b642e1e0b4677db7b8fc6150d480eb751
src/tests/mock_slave.cpp 50c04bff9e6f0f202af4c07b4036e021d3833ebf
Diff: https://reviews.apache.org/r/57882/diff/3/
Changes: https://reviews.apache.org/r/57882/diff/2-3/
Testing
-------
Testing details can be found at the end of this chain.
Thanks,
Greg Mann
Re: Review Request 57882: Added SecretGenerator injection to the
MockSlave.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57882/
-----------------------------------------------------------
(Updated March 23, 2017, 8:44 p.m.)
Review request for mesos, Anand Mazumdar and Vinod Kone.
Bugs: MESOS-6999
https://issues.apache.org/jira/browse/MESOS-6999
Repository: mesos
Description
-------
This patch adds an overload of `Slave::initialize` to the
mock slave for the purpose of injecting a secret generator.
Diffs (updated)
-----
src/tests/mock_slave.hpp 1acb961b642e1e0b4677db7b8fc6150d480eb751
src/tests/mock_slave.cpp 50c04bff9e6f0f202af4c07b4036e021d3833ebf
Diff: https://reviews.apache.org/r/57882/diff/2/
Changes: https://reviews.apache.org/r/57882/diff/1-2/
Testing
-------
Testing details can be found at the end of this chain.
Thanks,
Greg Mann