Re: Review Request 40224: Fix wrong flags infos in /state and /flags

[Joris Van Remoortere]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40224/#review141517
---



Closing this review due to inactivity. Please see our 
[guidelines](https://github.com/apache/mesos/blob/master/docs/reopening-reviews.md)
 for reopening reviews.

- Joris Van Remoortere


On Dec. 9, 2015, 2:20 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40224/
> ---
> 
> (Updated Dec. 9, 2015, 2:20 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and haosdent huang.
> 
> 
> Bugs: MESOS-3792
> https://issues.apache.org/jira/browse/MESOS-3792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix wrong flags infos in /state and /flags. We should convert protobuf type 
> flag to JSON object explicitly instead of relying on stringify
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 9097eda47558fb5306af5a61b464937d7ab34e83 
>   src/slave/http.cpp cef568d77da430b96e8f97487a8d0406dc0a0116 
> 
> Diff: https://reviews.apache.org/r/40224/diff/
> 
> 
> Testing
> ---
> 
> make & make check
> 
> ./mesos-master.sh --work_dir=/tmp/mesos --acls='{"register_frameworks": 
> [{"principals": { "type": "ANY" },"roles": { "values": ["a"] }}],"run_tasks": 
> [{"principals": { "values": ["a", "b"] },"users": { "values": ["c"] 
> }}],"shutdown_frameworks": [{"principals": { "values": ["a", "b"] 
> },"framework_principals": { "values": ["c"] }}]}'
> 
> curl http://master:5050/state
> curl http://master:5050/flag
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 40224: Fix wrong flags infos in /state and /flags

[haosdent huang]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40224/#review112741
---

Ship it!


Ship It!

- haosdent huang


On Dec. 9, 2015, 2:20 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40224/
> ---
> 
> (Updated Dec. 9, 2015, 2:20 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and haosdent huang.
> 
> 
> Bugs: MESOS-3792
> https://issues.apache.org/jira/browse/MESOS-3792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix wrong flags infos in /state and /flags. We should convert protobuf type 
> flag to JSON object explicitly instead of relying on stringify
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 9097eda47558fb5306af5a61b464937d7ab34e83 
>   src/slave/http.cpp cef568d77da430b96e8f97487a8d0406dc0a0116 
> 
> Diff: https://reviews.apache.org/r/40224/diff/
> 
> 
> Testing
> ---
> 
> make & make check
> 
> ./mesos-master.sh --work_dir=/tmp/mesos --acls='{"register_frameworks": 
> [{"principals": { "type": "ANY" },"roles": { "values": ["a"] }}],"run_tasks": 
> [{"principals": { "values": ["a", "b"] },"users": { "values": ["c"] 
> }}],"shutdown_frameworks": [{"principals": { "values": ["a", "b"] 
> },"framework_principals": { "values": ["c"] }}]}'
> 
> curl http://master:5050/state
> curl http://master:5050/flag
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 40224: Fix wrong flags infos in /state and /flags

[Mesos ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40224/#review109479
---


Patch looks great!

Reviews applied: [40224]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 9, 2015, 2:20 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40224/
> ---
> 
> (Updated Dec. 9, 2015, 2:20 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and haosdent huang.
> 
> 
> Bugs: MESOS-3792
> https://issues.apache.org/jira/browse/MESOS-3792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix wrong flags infos in /state and /flags. We should convert protobuf type 
> flag to JSON object explicitly instead of relying on stringify
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 9097eda47558fb5306af5a61b464937d7ab34e83 
>   src/slave/http.cpp cef568d77da430b96e8f97487a8d0406dc0a0116 
> 
> Diff: https://reviews.apache.org/r/40224/diff/
> 
> 
> Testing
> ---
> 
> make & make check
> 
> ./mesos-master.sh --work_dir=/tmp/mesos --acls='{"register_frameworks": 
> [{"principals": { "type": "ANY" },"roles": { "values": ["a"] }}],"run_tasks": 
> [{"principals": { "values": ["a", "b"] },"users": { "values": ["c"] 
> }}],"shutdown_frameworks": [{"principals": { "values": ["a", "b"] 
> },"framework_principals": { "values": ["c"] }}]}'
> 
> curl http://master:5050/state
> curl http://master:5050/flag
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 40224: Fix wrong flags infos in /state and /flags

[Jian Qiu]

> On Nov. 23, 2015, 4:08 p.m., haosdent huang wrote:
> > src/master/http.cpp, line 331
> > 
> >
> > Seems miss credentials, whitelist

These two flags returns as Path instead of protobuf.


- Jian


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40224/#review107604
---


On Nov. 23, 2015, 5:56 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40224/
> ---
> 
> (Updated Nov. 23, 2015, 5:56 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and haosdent huang.
> 
> 
> Bugs: MESOS-3792
> https://issues.apache.org/jira/browse/MESOS-3792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix wrong flags infos in /state and /flags. We should convert protobuf type 
> flag to JSON object explicitly instead of relying on stringify
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp b0bec97ee69413bb70c2673c4ae49e74988796bf 
> 
> Diff: https://reviews.apache.org/r/40224/diff/
> 
> 
> Testing
> ---
> 
> make & make check
> 
> ./mesos-master.sh --work_dir=/tmp/mesos --acls='{"register_frameworks": 
> [{"principals": { "type": "ANY" },"roles": { "values": ["a"] }}],"run_tasks": 
> [{"principals": { "values": ["a", "b"] },"users": { "values": ["c"] 
> }}],"shutdown_frameworks": [{"principals": { "values": ["a", "b"] 
> },"framework_principals": { "values": ["c"] }}]}'
> 
> curl http://master:5050/state
> curl http://master:5050/flag
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 40224: Fix wrong flags infos in /state and /flags

[Jian Qiu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40224/
---

(Updated Dec. 9, 2015, 2:20 a.m.)


Review request for mesos, Ben Mahler and haosdent huang.


Bugs: MESOS-3792
https://issues.apache.org/jira/browse/MESOS-3792


Repository: mesos


Description
---

Fix wrong flags infos in /state and /flags. We should convert protobuf type 
flag to JSON object explicitly instead of relying on stringify


Diffs (updated)
-

  src/master/http.cpp 9097eda47558fb5306af5a61b464937d7ab34e83 
  src/slave/http.cpp cef568d77da430b96e8f97487a8d0406dc0a0116 

Diff: https://reviews.apache.org/r/40224/diff/


Testing
---

make & make check

./mesos-master.sh --work_dir=/tmp/mesos --acls='{"register_frameworks": 
[{"principals": { "type": "ANY" },"roles": { "values": ["a"] }}],"run_tasks": 
[{"principals": { "values": ["a", "b"] },"users": { "values": ["c"] 
}}],"shutdown_frameworks": [{"principals": { "values": ["a", "b"] 
},"framework_principals": { "values": ["c"] }}]}'

curl http://master:5050/state
curl http://master:5050/flag


Thanks,

Jian Qiu

Re: Review Request 40224: Fix wrong flags infos in /state and /flags

[Jian Qiu]

> On Nov. 23, 2015, 4:08 p.m., haosdent huang wrote:
> > I think keep the sync of these files when flags add or delete Protobuf 
> > fields a bit hard. Is it possible to change flags stringify protobuf 
> > objects as json string

Thanks hasdent. I do not think we can overload or have a specialized stringify 
for google::protobuf::Message since it is an abstract class. How about 
rewriting the operator<< overload for each protobuf flag? We can change 
https://github.com/apache/mesos/blob/master/include/mesos/authorizer/authorizer.hpp#L138
 to "return stream << JSON::protobuf(acls)". However, it still needs change for 
each protobuf flag. Any thought?


- Jian


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40224/#review107604
---


On Nov. 23, 2015, 5:56 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40224/
> ---
> 
> (Updated Nov. 23, 2015, 5:56 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and haosdent huang.
> 
> 
> Bugs: MESOS-3792
> https://issues.apache.org/jira/browse/MESOS-3792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix wrong flags infos in /state and /flags. We should convert protobuf type 
> flag to JSON object explicitly instead of relying on stringify
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp b0bec97ee69413bb70c2673c4ae49e74988796bf 
> 
> Diff: https://reviews.apache.org/r/40224/diff/
> 
> 
> Testing
> ---
> 
> make & make check
> 
> ./mesos-master.sh --work_dir=/tmp/mesos --acls='{"register_frameworks": 
> [{"principals": { "type": "ANY" },"roles": { "values": ["a"] }}],"run_tasks": 
> [{"principals": { "values": ["a", "b"] },"users": { "values": ["c"] 
> }}],"shutdown_frameworks": [{"principals": { "values": ["a", "b"] 
> },"framework_principals": { "values": ["c"] }}]}'
> 
> curl http://master:5050/state
> curl http://master:5050/flag
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 40224: Fix wrong flags infos in /state and /flags

[haosdent huang]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40224/#review107604
---


I think keep the sync of these files when flags add or delete Protobuf fields a 
bit hard. Is it possible to change flags stringify protobuf objects as json 
string


src/master/http.cpp (line 331)


Seems miss credentials, whitelist



src/master/http.cpp (line 625)


Seems slave also have similar things? Do we need also update http.cpp in 
slave?


- haosdent huang


On Nov. 23, 2015, 5:56 a.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40224/
> ---
> 
> (Updated Nov. 23, 2015, 5:56 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and haosdent huang.
> 
> 
> Bugs: MESOS-3792
> https://issues.apache.org/jira/browse/MESOS-3792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix wrong flags infos in /state and /flags. We should convert protobuf type 
> flag to JSON object explicitly instead of relying on stringify
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp b0bec97ee69413bb70c2673c4ae49e74988796bf 
> 
> Diff: https://reviews.apache.org/r/40224/diff/
> 
> 
> Testing
> ---
> 
> make & make check
> 
> ./mesos-master.sh --work_dir=/tmp/mesos --acls='{"register_frameworks": 
> [{"principals": { "type": "ANY" },"roles": { "values": ["a"] }}],"run_tasks": 
> [{"principals": { "values": ["a", "b"] },"users": { "values": ["c"] 
> }}],"shutdown_frameworks": [{"principals": { "values": ["a", "b"] 
> },"framework_principals": { "values": ["c"] }}]}'
> 
> curl http://master:5050/state
> curl http://master:5050/flag
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Review Request 40224: Fix wrong flags infos in /state and /flags

[Jian Qiu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40224/
---

Review request for mesos, Ben Mahler and haosdent huang.


Bugs: MESOS-3792
https://issues.apache.org/jira/browse/MESOS-3792


Repository: mesos


Description
---

Fix wrong flags infos in /state and /flags. We should convert protobuf type 
flag to JSON object explicitly instead of relying on stringify


Diffs
-

  src/master/http.cpp b0bec97ee69413bb70c2673c4ae49e74988796bf 

Diff: https://reviews.apache.org/r/40224/diff/


Testing
---

make & make check

./mesos-master.sh --work_dir=/tmp/mesos --acls='{"register_frameworks": 
[{"principals": { "type": "ANY" },"roles": { "values": ["a"] }}],"run_tasks": 
[{"principals": { "values": ["a", "b"] },"users": { "values": ["c"] 
}}],"shutdown_frameworks": [{"principals": { "values": ["a", "b"] 
},"framework_principals": { "values": ["c"] }}]}'

curl http://master:5050/state
curl http://master:5050/flag


Thanks,

Jian Qiu