You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Vinod Kone <vi...@gmail.com> on 2014/06/17 20:46:14 UTC
Review Request 22690: Added flags::parse() overload for ACLs protobuf.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22690/
-----------------------------------------------------------
Review request for mesos, Ben Mahler and Jiang Yan Xu.
Bugs: MESOS-1499
https://issues.apache.org/jira/browse/MESOS-1499
Repository: mesos-git
Description
-------
See summary.
Diffs
-----
src/Makefile.am 3e623cc0137ea4ae9113f9b47a4fc88281ee96c1
src/authorizer/authorizer.hpp 43e1c3b177c3ea6f4099afbeb0415579b8284cac
src/authorizer/authorizer.cpp c46b31d0f963311e5d646fa16bef51e8ad19c7f2
src/common/parse.hpp PRE-CREATION
src/common/type_utils.hpp 784a808a5e0b78dab24a4806d6f1f9491a2d6d44
src/master/flags.hpp 780e219e81e3df57da197784f67deab2b0de4382
src/tests/master_authorization_tests.cpp 661d67e50c828b1e5e806f68719aef020455abf2
src/tests/mesos.cpp 98a7c3895343b1bc84c13e54d12905de3b431338
Diff: https://reviews.apache.org/r/22690/diff/
Testing
-------
make check
Thanks,
Vinod Kone
Re: Review Request 22690: Added flags::parse() overload for ACLs protobuf.
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On June 17, 2014, 8:11 p.m., Benjamin Hindman wrote:
> > src/common/type_utils.hpp, line 121
> > <https://reviews.apache.org/r/22690/diff/1/?file=611759#file611759line121>
> >
> > Why not as JSON?
>
> Vinod Kone wrote:
> Seems weird to print JSON for ACLs protobuf when all other protobufs in this file are printed via debug string? FWIW, protobuf's debug string has better formatting/readability than our JSON stringify. Once we have JSON formatter/pprint we could convert all outputstream operators in this file to use JSON. Thoughts?
>
> Debug string output:
> --------------------
> run_tasks {
> principals {
> type: ANY
> }
> users {
> values: "guest"
> }
> }
>
> JSON stringify
> ---------------
> {"permissive":true,"run_tasks":[{"principals":{"type":"ANY"},"users":{"type":"SOME","values":["guest"]}}]}
>
>
SGTM!
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22690/#review45976
-----------------------------------------------------------
On June 17, 2014, 8:37 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22690/
> -----------------------------------------------------------
>
> (Updated June 17, 2014, 8:37 p.m.)
>
>
> Review request for mesos, Ben Mahler and Jiang Yan Xu.
>
>
> Bugs: MESOS-1499
> https://issues.apache.org/jira/browse/MESOS-1499
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/Makefile.am 3e623cc0137ea4ae9113f9b47a4fc88281ee96c1
> src/authorizer/authorizer.hpp 43e1c3b177c3ea6f4099afbeb0415579b8284cac
> src/authorizer/authorizer.cpp c46b31d0f963311e5d646fa16bef51e8ad19c7f2
> src/common/parse.hpp PRE-CREATION
> src/common/type_utils.hpp 784a808a5e0b78dab24a4806d6f1f9491a2d6d44
> src/master/flags.hpp 780e219e81e3df57da197784f67deab2b0de4382
> src/tests/master_authorization_tests.cpp 661d67e50c828b1e5e806f68719aef020455abf2
> src/tests/mesos.cpp 98a7c3895343b1bc84c13e54d12905de3b431338
>
> Diff: https://reviews.apache.org/r/22690/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 22690: Added flags::parse() overload for ACLs protobuf.
Posted by Vinod Kone <vi...@gmail.com>.
> On June 17, 2014, 8:11 p.m., Benjamin Hindman wrote:
> > src/common/type_utils.hpp, line 121
> > <https://reviews.apache.org/r/22690/diff/1/?file=611759#file611759line121>
> >
> > Why not as JSON?
Seems weird to print JSON for ACLs protobuf when all other protobufs in this file are printed via debug string? FWIW, protobuf's debug string has better formatting/readability than our JSON stringify. Once we have JSON formatter/pprint we could convert all outputstream operators in this file to use JSON. Thoughts?
Debug string output:
--------------------
run_tasks {
principals {
type: ANY
}
users {
values: "guest"
}
}
JSON stringify
---------------
{"permissive":true,"run_tasks":[{"principals":{"type":"ANY"},"users":{"type":"SOME","values":["guest"]}}]}
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22690/#review45976
-----------------------------------------------------------
On June 17, 2014, 6:46 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22690/
> -----------------------------------------------------------
>
> (Updated June 17, 2014, 6:46 p.m.)
>
>
> Review request for mesos, Ben Mahler and Jiang Yan Xu.
>
>
> Bugs: MESOS-1499
> https://issues.apache.org/jira/browse/MESOS-1499
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/Makefile.am 3e623cc0137ea4ae9113f9b47a4fc88281ee96c1
> src/authorizer/authorizer.hpp 43e1c3b177c3ea6f4099afbeb0415579b8284cac
> src/authorizer/authorizer.cpp c46b31d0f963311e5d646fa16bef51e8ad19c7f2
> src/common/parse.hpp PRE-CREATION
> src/common/type_utils.hpp 784a808a5e0b78dab24a4806d6f1f9491a2d6d44
> src/master/flags.hpp 780e219e81e3df57da197784f67deab2b0de4382
> src/tests/master_authorization_tests.cpp 661d67e50c828b1e5e806f68719aef020455abf2
> src/tests/mesos.cpp 98a7c3895343b1bc84c13e54d12905de3b431338
>
> Diff: https://reviews.apache.org/r/22690/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 22690: Added flags::parse() overload for ACLs protobuf.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22690/#review45976
-----------------------------------------------------------
Ship it!
src/common/parse.hpp
<https://reviews.apache.org/r/22690/#comment81140>
Oops!
src/common/type_utils.hpp
<https://reviews.apache.org/r/22690/#comment81141>
Why not as JSON?
- Benjamin Hindman
On June 17, 2014, 6:46 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22690/
> -----------------------------------------------------------
>
> (Updated June 17, 2014, 6:46 p.m.)
>
>
> Review request for mesos, Ben Mahler and Jiang Yan Xu.
>
>
> Bugs: MESOS-1499
> https://issues.apache.org/jira/browse/MESOS-1499
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/Makefile.am 3e623cc0137ea4ae9113f9b47a4fc88281ee96c1
> src/authorizer/authorizer.hpp 43e1c3b177c3ea6f4099afbeb0415579b8284cac
> src/authorizer/authorizer.cpp c46b31d0f963311e5d646fa16bef51e8ad19c7f2
> src/common/parse.hpp PRE-CREATION
> src/common/type_utils.hpp 784a808a5e0b78dab24a4806d6f1f9491a2d6d44
> src/master/flags.hpp 780e219e81e3df57da197784f67deab2b0de4382
> src/tests/master_authorization_tests.cpp 661d67e50c828b1e5e806f68719aef020455abf2
> src/tests/mesos.cpp 98a7c3895343b1bc84c13e54d12905de3b431338
>
> Diff: https://reviews.apache.org/r/22690/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 22690: Added flags::parse() overload for ACLs protobuf.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22690/#review45984
-----------------------------------------------------------
Ship it!
src/common/parse.hpp
<https://reviews.apache.org/r/22690/#comment81150>
Not keeping the "Invalid ACLs format: " prefix?
- Jiang Yan Xu
On June 17, 2014, 1:37 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22690/
> -----------------------------------------------------------
>
> (Updated June 17, 2014, 1:37 p.m.)
>
>
> Review request for mesos, Ben Mahler and Jiang Yan Xu.
>
>
> Bugs: MESOS-1499
> https://issues.apache.org/jira/browse/MESOS-1499
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/Makefile.am 3e623cc0137ea4ae9113f9b47a4fc88281ee96c1
> src/authorizer/authorizer.hpp 43e1c3b177c3ea6f4099afbeb0415579b8284cac
> src/authorizer/authorizer.cpp c46b31d0f963311e5d646fa16bef51e8ad19c7f2
> src/common/parse.hpp PRE-CREATION
> src/common/type_utils.hpp 784a808a5e0b78dab24a4806d6f1f9491a2d6d44
> src/master/flags.hpp 780e219e81e3df57da197784f67deab2b0de4382
> src/tests/master_authorization_tests.cpp 661d67e50c828b1e5e806f68719aef020455abf2
> src/tests/mesos.cpp 98a7c3895343b1bc84c13e54d12905de3b431338
>
> Diff: https://reviews.apache.org/r/22690/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 22690: Added flags::parse() overload for ACLs protobuf.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22690/#review46035
-----------------------------------------------------------
Bad patch!
Reviews applied: [22690]
Failed command: git apply --index 22690.patch
Error:
error: patch failed: src/Makefile.am:331
error: src/Makefile.am: patch does not apply
error: patch failed: src/authorizer/authorizer.hpp:43
error: src/authorizer/authorizer.hpp: patch does not apply
error: patch failed: src/authorizer/authorizer.cpp:233
error: src/authorizer/authorizer.cpp: patch does not apply
error: src/common/parse.hpp: already exists in index
error: patch failed: src/master/flags.hpp:25
error: src/master/flags.hpp: patch does not apply
error: patch failed: src/tests/master_authorization_tests.cpp:77
error: src/tests/master_authorization_tests.cpp: patch does not apply
error: patch failed: src/tests/mesos.cpp:102
error: src/tests/mesos.cpp: patch does not apply
- Mesos ReviewBot
On June 17, 2014, 8:37 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22690/
> -----------------------------------------------------------
>
> (Updated June 17, 2014, 8:37 p.m.)
>
>
> Review request for mesos, Ben Mahler and Jiang Yan Xu.
>
>
> Bugs: MESOS-1499
> https://issues.apache.org/jira/browse/MESOS-1499
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/Makefile.am 3e623cc0137ea4ae9113f9b47a4fc88281ee96c1
> src/authorizer/authorizer.hpp 43e1c3b177c3ea6f4099afbeb0415579b8284cac
> src/authorizer/authorizer.cpp c46b31d0f963311e5d646fa16bef51e8ad19c7f2
> src/common/parse.hpp PRE-CREATION
> src/common/type_utils.hpp 784a808a5e0b78dab24a4806d6f1f9491a2d6d44
> src/master/flags.hpp 780e219e81e3df57da197784f67deab2b0de4382
> src/tests/master_authorization_tests.cpp 661d67e50c828b1e5e806f68719aef020455abf2
> src/tests/mesos.cpp 98a7c3895343b1bc84c13e54d12905de3b431338
>
> Diff: https://reviews.apache.org/r/22690/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 22690: Added flags::parse() overload for ACLs protobuf.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22690/#review45995
-----------------------------------------------------------
Great!
- Ben Mahler
On June 17, 2014, 8:37 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22690/
> -----------------------------------------------------------
>
> (Updated June 17, 2014, 8:37 p.m.)
>
>
> Review request for mesos, Ben Mahler and Jiang Yan Xu.
>
>
> Bugs: MESOS-1499
> https://issues.apache.org/jira/browse/MESOS-1499
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/Makefile.am 3e623cc0137ea4ae9113f9b47a4fc88281ee96c1
> src/authorizer/authorizer.hpp 43e1c3b177c3ea6f4099afbeb0415579b8284cac
> src/authorizer/authorizer.cpp c46b31d0f963311e5d646fa16bef51e8ad19c7f2
> src/common/parse.hpp PRE-CREATION
> src/common/type_utils.hpp 784a808a5e0b78dab24a4806d6f1f9491a2d6d44
> src/master/flags.hpp 780e219e81e3df57da197784f67deab2b0de4382
> src/tests/master_authorization_tests.cpp 661d67e50c828b1e5e806f68719aef020455abf2
> src/tests/mesos.cpp 98a7c3895343b1bc84c13e54d12905de3b431338
>
> Diff: https://reviews.apache.org/r/22690/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 22690: Added flags::parse() overload for ACLs protobuf.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22690/
-----------------------------------------------------------
(Updated June 17, 2014, 8:37 p.m.)
Review request for mesos, Ben Mahler and Jiang Yan Xu.
Changes
-------
benh's.
Bugs: MESOS-1499
https://issues.apache.org/jira/browse/MESOS-1499
Repository: mesos-git
Description
-------
See summary.
Diffs (updated)
-----
src/Makefile.am 3e623cc0137ea4ae9113f9b47a4fc88281ee96c1
src/authorizer/authorizer.hpp 43e1c3b177c3ea6f4099afbeb0415579b8284cac
src/authorizer/authorizer.cpp c46b31d0f963311e5d646fa16bef51e8ad19c7f2
src/common/parse.hpp PRE-CREATION
src/common/type_utils.hpp 784a808a5e0b78dab24a4806d6f1f9491a2d6d44
src/master/flags.hpp 780e219e81e3df57da197784f67deab2b0de4382
src/tests/master_authorization_tests.cpp 661d67e50c828b1e5e806f68719aef020455abf2
src/tests/mesos.cpp 98a7c3895343b1bc84c13e54d12905de3b431338
Diff: https://reviews.apache.org/r/22690/diff/
Testing
-------
make check
Thanks,
Vinod Kone