You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benno Evers <be...@mesosphere.com> on 2018/08/14 14:20:15 UTC
Review Request 68343: Moved state serialization into separate
function.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68343/
-----------------------------------------------------------
Review request for mesos and Alexander Rukletsov.
Repository: mesos
Description
-------
In preparation for the changes in the subsequent commit,
the serialization logic was moved from an inner lambda
to a separate function.
Diffs
-----
src/master/http.cpp e2773ed78d3d07da13029a4d75143f062d97c852
src/master/master.hpp 85ef14c1cc72180b746a5f4375769b653cbe511d
Diff: https://reviews.apache.org/r/68343/diff/1/
Testing
-------
Thanks,
Benno Evers
Re: Review Request 68343: Moved state serialization into separate
function.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68343/#review207410
-----------------------------------------------------------
src/master/http.cpp
Lines 2876-2878 (patched)
<https://reviews.apache.org/r/68343/#comment290800>
Let's maybe make it a method in a to-be-introduced `Master::Http::ReadOnlyHandler` class. This way you hide the `const Master*` inside the `ReadOnlyHandler` instance and keep the endpoint handler geographically closer to other handler code.
- Alexander Rukletsov
On Aug. 14, 2018, 2:20 p.m., Benno Evers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68343/
> -----------------------------------------------------------
>
> (Updated Aug. 14, 2018, 2:20 p.m.)
>
>
> Review request for mesos and Alexander Rukletsov.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> In preparation for the changes in the subsequent commit,
> the serialization logic was moved from an inner lambda
> to a separate function.
>
>
> Diffs
> -----
>
> src/master/http.cpp e2773ed78d3d07da13029a4d75143f062d97c852
> src/master/master.hpp 85ef14c1cc72180b746a5f4375769b653cbe511d
>
>
> Diff: https://reviews.apache.org/r/68343/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Benno Evers
>
>
Re: Review Request 68343: Moved state serialization into separate
function.
Posted by Benno Evers <be...@mesosphere.com>.
> On Aug. 20, 2018, 9:43 a.m., Alexander Rukletsov wrote:
> > src/master/http.cpp
> > Line 3031 (original), 3030-3031 (patched)
> > <https://reviews.apache.org/r/68343/diff/2/?file=2074712#file2074712line3039>
> >
> > Is pass-by-value intentional?
I think it is required by `async()`, but I'll need to double-check.
- Benno
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68343/#review207605
-----------------------------------------------------------
On Aug. 20, 2018, 2:08 p.m., Benno Evers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68343/
> -----------------------------------------------------------
>
> (Updated Aug. 20, 2018, 2:08 p.m.)
>
>
> Review request for mesos and Alexander Rukletsov.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> In preparation for the changes in the subsequent commit,
> the serialization logic was moved from an inner lambda
> to a separate function.
>
>
> Diffs
> -----
>
> src/master/http.cpp e2773ed78d3d07da13029a4d75143f062d97c852
> src/master/master.hpp 36994a18a8e9092c0a46834f56ce3d9448e2bca3
>
>
> Diff: https://reviews.apache.org/r/68343/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Benno Evers
>
>
Re: Review Request 68343: Moved state serialization into separate
function.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68343/#review207605
-----------------------------------------------------------
src/master/http.cpp
Line 3031 (original), 3030 (patched)
<https://reviews.apache.org/r/68343/#comment291054>
s/] (/](
src/master/http.cpp
Line 3031 (original), 3030-3031 (patched)
<https://reviews.apache.org/r/68343/#comment291055>
Is pass-by-value intentional?
src/master/master.hpp
Lines 1389-1390 (patched)
<https://reviews.apache.org/r/68343/#comment291056>
corresponding what?
src/master/master.hpp
Lines 1398 (patched)
<https://reviews.apache.org/r/68343/#comment291058>
On the previous line please.
- Alexander Rukletsov
On Aug. 17, 2018, 7:06 p.m., Benno Evers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68343/
> -----------------------------------------------------------
>
> (Updated Aug. 17, 2018, 7:06 p.m.)
>
>
> Review request for mesos and Alexander Rukletsov.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> In preparation for the changes in the subsequent commit,
> the serialization logic was moved from an inner lambda
> to a separate function.
>
>
> Diffs
> -----
>
> src/master/http.cpp e2773ed78d3d07da13029a4d75143f062d97c852
> src/master/master.hpp 85ef14c1cc72180b746a5f4375769b653cbe511d
>
>
> Diff: https://reviews.apache.org/r/68343/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Benno Evers
>
>
Re: Review Request 68343: Moved state serialization into separate
function.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68343/#review207607
-----------------------------------------------------------
src/master/master.hpp
Lines 1402-1403 (patched)
<https://reviews.apache.org/r/68343/#comment291061>
Indentation
- Alexander Rukletsov
On Aug. 17, 2018, 7:06 p.m., Benno Evers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68343/
> -----------------------------------------------------------
>
> (Updated Aug. 17, 2018, 7:06 p.m.)
>
>
> Review request for mesos and Alexander Rukletsov.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> In preparation for the changes in the subsequent commit,
> the serialization logic was moved from an inner lambda
> to a separate function.
>
>
> Diffs
> -----
>
> src/master/http.cpp e2773ed78d3d07da13029a4d75143f062d97c852
> src/master/master.hpp 85ef14c1cc72180b746a5f4375769b653cbe511d
>
>
> Diff: https://reviews.apache.org/r/68343/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Benno Evers
>
>
Re: Review Request 68343: Moved state serialization into separate
function.
Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68343/
-----------------------------------------------------------
(Updated Aug. 21, 2018, 6:47 a.m.)
Review request for mesos and Alexander Rukletsov.
Bugs: MESOS-9158
https://issues.apache.org/jira/browse/MESOS-9158
Repository: mesos
Description
-------
In preparation for the changes in the subsequent commit,
the serialization logic was moved from an inner lambda
to a separate function.
Diffs
-----
src/master/http.cpp e2773ed78d3d07da13029a4d75143f062d97c852
src/master/master.hpp 36994a18a8e9092c0a46834f56ce3d9448e2bca3
Diff: https://reviews.apache.org/r/68343/diff/4/
Testing (updated)
-------
See https://reviews.apache.org/r/68296/
Thanks,
Benno Evers
Re: Review Request 68343: Moved state serialization into separate
function.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68343/#review207648
-----------------------------------------------------------
Ship it!
I'll do the last tweaks and commit this for you.
src/master/http.cpp
Line 2874 (original), 2874 (patched)
<https://reviews.apache.org/r/68343/#comment291084>
Let's move the difinition around to follow the flow in .hpp. Alternatively we can also put them all, say, in "master/readonly_handler.cpp". What'd you say?
src/master/master.hpp
Lines 1388-1389 (patched)
<https://reviews.apache.org/r/68343/#comment291083>
We have an unwritten agreement to add a black line between paragraphs in comments.
- Alexander Rukletsov
On Aug. 20, 2018, 6:34 p.m., Benno Evers wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68343/
> -----------------------------------------------------------
>
> (Updated Aug. 20, 2018, 6:34 p.m.)
>
>
> Review request for mesos and Alexander Rukletsov.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> In preparation for the changes in the subsequent commit,
> the serialization logic was moved from an inner lambda
> to a separate function.
>
>
> Diffs
> -----
>
> src/master/http.cpp e2773ed78d3d07da13029a4d75143f062d97c852
> src/master/master.hpp 36994a18a8e9092c0a46834f56ce3d9448e2bca3
>
>
> Diff: https://reviews.apache.org/r/68343/diff/4/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Benno Evers
>
>
Re: Review Request 68343: Moved state serialization into separate
function.
Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68343/
-----------------------------------------------------------
(Updated Aug. 20, 2018, 6:34 p.m.)
Review request for mesos and Alexander Rukletsov.
Changes
-------
Change args to const-ref.
Repository: mesos
Description
-------
In preparation for the changes in the subsequent commit,
the serialization logic was moved from an inner lambda
to a separate function.
Diffs (updated)
-----
src/master/http.cpp e2773ed78d3d07da13029a4d75143f062d97c852
src/master/master.hpp 36994a18a8e9092c0a46834f56ce3d9448e2bca3
Diff: https://reviews.apache.org/r/68343/diff/4/
Changes: https://reviews.apache.org/r/68343/diff/3-4/
Testing
-------
Thanks,
Benno Evers
Re: Review Request 68343: Moved state serialization into separate
function.
Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68343/
-----------------------------------------------------------
(Updated Aug. 20, 2018, 2:08 p.m.)
Review request for mesos and Alexander Rukletsov.
Repository: mesos
Description
-------
In preparation for the changes in the subsequent commit,
the serialization logic was moved from an inner lambda
to a separate function.
Diffs (updated)
-----
src/master/http.cpp e2773ed78d3d07da13029a4d75143f062d97c852
src/master/master.hpp 36994a18a8e9092c0a46834f56ce3d9448e2bca3
Diff: https://reviews.apache.org/r/68343/diff/3/
Changes: https://reviews.apache.org/r/68343/diff/2-3/
Testing
-------
Thanks,
Benno Evers
Re: Review Request 68343: Moved state serialization into separate
function.
Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68343/
-----------------------------------------------------------
(Updated Aug. 17, 2018, 7:06 p.m.)
Review request for mesos and Alexander Rukletsov.
Repository: mesos
Description
-------
In preparation for the changes in the subsequent commit,
the serialization logic was moved from an inner lambda
to a separate function.
Diffs (updated)
-----
src/master/http.cpp e2773ed78d3d07da13029a4d75143f062d97c852
src/master/master.hpp 85ef14c1cc72180b746a5f4375769b653cbe511d
Diff: https://reviews.apache.org/r/68343/diff/2/
Changes: https://reviews.apache.org/r/68343/diff/1-2/
Testing
-------
Thanks,
Benno Evers