Re: Review Request 37024: Add an endpoint that exposes component version.
On Aug. 3, 2015, 7:37 p.m., Ben Mahler wrote:
src/master/http.cpp, lines 526-542
https://reviews.apache.org/r/37024/diff/1/?file=1027286#file1027286line526
Could you include these fields as well? They capture version /
build information. Once you do, perhaps it is worth adding a
'`JSON::Object version()`' helper in common/http.hpp.
Marco Massenzio wrote:
great point.
also wondering if, at this point, it would make sense to have a Protobuf
`VersionInfo` (that we could also use to replace the `version` string in
`MasterInfo`) which can then be used everywhere (and gets auto-converted to
JSON)?
Thank you @marco, it is OK for me. What's @bmahler opinion? :-P
- haosdent
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37024/#review93947
---
On Aug. 2, 2015, 10:16 a.m., haosdent huang wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37024/
---
(Updated Aug. 2, 2015, 10:16 a.m.)
Review request for mesos and Ben Mahler.
Bugs: MESOS-1841
https://issues.apache.org/jira/browse/MESOS-1841
Repository: mesos
Description
---
Add an endpoint that exposes component version.
Diffs
-
src/master/http.cpp 3772e39015a22655dcad00ad844dc5ddc90db43f
src/master/master.hpp ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc
src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544
src/slave/http.cpp b0fe5f520dfca156548ba8c436d42fc432223f3d
src/slave/slave.hpp 41d09497be313819a9c78361b8595f6f26dc8460
src/slave/slave.cpp 6b21db973dc95dd5eb2238eebe697db9dd063ef1
Diff: https://reviews.apache.org/r/37024/diff/
Testing
---
Manual test result:
```
$ curl http://localhost:5050/slave(1)/version.json 2/dev/null|jq .
{
version: 0.24.0
}
```
```
$ curl http://localhost:5050/master/version.json 2/dev/null|jq .
{
version: 0.24.0
}
```
Thanks,
haosdent huang
Re: Review Request 37024: Add an endpoint that exposes component version.
On Aug. 8, 2015, 9:19 a.m., Marco Massenzio wrote:
src/master/http.cpp, line 1185
https://reviews.apache.org/r/37024/diff/1/?file=1027286#file1027286line1185
can you please double-check this is consistent with what we add to
`MasterInfo` in the master's `initialize()` method?
I think it is, but not sure.
(sorry, I don't have the code at hand on this Mac)
In master's `initialize()` method,
```cpp
info_.set_version(MESOS_VERSION);
```
So I think they are same. Should I change to use `info.get_version` here?
- haosdent
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37024/#review94628
---
On Aug. 2, 2015, 10:16 a.m., haosdent huang wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37024/
---
(Updated Aug. 2, 2015, 10:16 a.m.)
Review request for mesos and Ben Mahler.
Bugs: MESOS-1841
https://issues.apache.org/jira/browse/MESOS-1841
Repository: mesos
Description
---
Add an endpoint that exposes component version.
Diffs
-
src/master/http.cpp 3772e39015a22655dcad00ad844dc5ddc90db43f
src/master/master.hpp ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc
src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544
src/slave/http.cpp b0fe5f520dfca156548ba8c436d42fc432223f3d
src/slave/slave.hpp 41d09497be313819a9c78361b8595f6f26dc8460
src/slave/slave.cpp 6b21db973dc95dd5eb2238eebe697db9dd063ef1
Diff: https://reviews.apache.org/r/37024/diff/
Testing
---
Manual test result:
```
$ curl http://localhost:5050/slave(1)/version.json 2/dev/null|jq .
{
version: 0.24.0
}
```
```
$ curl http://localhost:5050/master/version.json 2/dev/null|jq .
{
version: 0.24.0
}
```
Thanks,
haosdent huang
Re: Review Request 37024: Add an endpoint that exposes component version.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37024/#review94628
---
src/master/http.cpp (line 1185)
https://reviews.apache.org/r/37024/#comment149214
can you please double-check this is consistent with what we add to
`MasterInfo` in the master's `initialize()` method?
I think it is, but not sure.
(sorry, I don't have the code at hand on this Mac)
- Marco Massenzio
On Aug. 2, 2015, 10:16 a.m., haosdent huang wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37024/
---
(Updated Aug. 2, 2015, 10:16 a.m.)
Review request for mesos and Ben Mahler.
Bugs: MESOS-1841
https://issues.apache.org/jira/browse/MESOS-1841
Repository: mesos
Description
---
Add an endpoint that exposes component version.
Diffs
-
src/master/http.cpp 3772e39015a22655dcad00ad844dc5ddc90db43f
src/master/master.hpp ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc
src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544
src/slave/http.cpp b0fe5f520dfca156548ba8c436d42fc432223f3d
src/slave/slave.hpp 41d09497be313819a9c78361b8595f6f26dc8460
src/slave/slave.cpp 6b21db973dc95dd5eb2238eebe697db9dd063ef1
Diff: https://reviews.apache.org/r/37024/diff/
Testing
---
Manual test result:
```
$ curl http://localhost:5050/slave(1)/version.json 2/dev/null|jq .
{
version: 0.24.0
}
```
```
$ curl http://localhost:5050/master/version.json 2/dev/null|jq .
{
version: 0.24.0
}
```
Thanks,
haosdent huang
Re: Review Request 37024: Add an endpoint that exposes component version.
On Aug. 3, 2015, 7:37 p.m., Ben Mahler wrote:
src/master/http.cpp, lines 526-542
https://reviews.apache.org/r/37024/diff/1/?file=1027286#file1027286line526
Could you include these fields as well? They capture version /
build information. Once you do, perhaps it is worth adding a
'`JSON::Object version()`' helper in common/http.hpp.
great point.
also wondering if, at this point, it would make sense to have a Protobuf
`VersionInfo` (that we could also use to replace the `version` string in
`MasterInfo`) which can then be used everywhere (and gets auto-converted to
JSON)?
- Marco
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37024/#review93947
---
On Aug. 2, 2015, 10:16 a.m., haosdent huang wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37024/
---
(Updated Aug. 2, 2015, 10:16 a.m.)
Review request for mesos and Ben Mahler.
Bugs: MESOS-1841
https://issues.apache.org/jira/browse/MESOS-1841
Repository: mesos
Description
---
Add an endpoint that exposes component version.
Diffs
-
src/master/http.cpp 3772e39015a22655dcad00ad844dc5ddc90db43f
src/master/master.hpp ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc
src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544
src/slave/http.cpp b0fe5f520dfca156548ba8c436d42fc432223f3d
src/slave/slave.hpp 41d09497be313819a9c78361b8595f6f26dc8460
src/slave/slave.cpp 6b21db973dc95dd5eb2238eebe697db9dd063ef1
Diff: https://reviews.apache.org/r/37024/diff/
Testing
---
Manual test result:
```
$ curl http://localhost:5050/slave(1)/version.json 2/dev/null|jq .
{
version: 0.24.0
}
```
```
$ curl http://localhost:5050/master/version.json 2/dev/null|jq .
{
version: 0.24.0
}
```
Thanks,
haosdent huang
Re: Review Request 37024: Add an endpoint that exposes component version.
On Aug. 3, 2015, 7:37 p.m., Ben Mahler wrote:
Thanks! Let's include the git / build information as well.
Another question, how are you planning to add this to the scheduler driver
and executor driver? In these cases, it is likely better to create a
'Version' Process with process id version that routes an endpoint at /
(so just /version should route to this). If we go with this approach, we
just have to start a 'Version' Process from the master/slave entrypoints
and the drivers' initialization.
Thank you very much. Let me use VersionProcess
- haosdent
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37024/#review93947
---
On Aug. 2, 2015, 10:16 a.m., haosdent huang wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37024/
---
(Updated Aug. 2, 2015, 10:16 a.m.)
Review request for mesos and Ben Mahler.
Bugs: MESOS-1841
https://issues.apache.org/jira/browse/MESOS-1841
Repository: mesos
Description
---
Add an endpoint that exposes component version.
Diffs
-
src/master/http.cpp 3772e39015a22655dcad00ad844dc5ddc90db43f
src/master/master.hpp ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc
src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544
src/slave/http.cpp b0fe5f520dfca156548ba8c436d42fc432223f3d
src/slave/slave.hpp 41d09497be313819a9c78361b8595f6f26dc8460
src/slave/slave.cpp 6b21db973dc95dd5eb2238eebe697db9dd063ef1
Diff: https://reviews.apache.org/r/37024/diff/
Testing
---
Manual test result:
```
$ curl http://localhost:5050/slave(1)/version.json 2/dev/null|jq .
{
version: 0.24.0
}
```
```
$ curl http://localhost:5050/master/version.json 2/dev/null|jq .
{
version: 0.24.0
}
```
Thanks,
haosdent huang
Re: Review Request 37024: Add an endpoint that exposes component version.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37024/#review93947
---
Thanks! Let's include the git / build information as well.
Another question, how are you planning to add this to the scheduler driver and
executor driver? In these cases, it is likely better to create a 'Version'
Process with process id version that routes an endpoint at / (so just
/version should route to this). If we go with this approach, we just have to
start a 'Version' Process from the master/slave entrypoints and the drivers'
initialization.
src/master/http.cpp (lines 526 - 542)
https://reviews.apache.org/r/37024/#comment148381
Could you include these fields as well? They capture version / build
information. Once you do, perhaps it is worth adding a '`JSON::Object
version()`' helper in common/http.hpp.
- Ben Mahler
On Aug. 2, 2015, 10:16 a.m., haosdent huang wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37024/
---
(Updated Aug. 2, 2015, 10:16 a.m.)
Review request for mesos and Ben Mahler.
Bugs: MESOS-1841
https://issues.apache.org/jira/browse/MESOS-1841
Repository: mesos
Description
---
Add an endpoint that exposes component version.
Diffs
-
src/master/http.cpp 3772e39015a22655dcad00ad844dc5ddc90db43f
src/master/master.hpp ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc
src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544
src/slave/http.cpp b0fe5f520dfca156548ba8c436d42fc432223f3d
src/slave/slave.hpp 41d09497be313819a9c78361b8595f6f26dc8460
src/slave/slave.cpp 6b21db973dc95dd5eb2238eebe697db9dd063ef1
Diff: https://reviews.apache.org/r/37024/diff/
Testing
---
Manual test result:
```
$ curl http://localhost:5050/slave(1)/version.json 2/dev/null|jq .
{
version: 0.24.0
}
```
```
$ curl http://localhost:5050/master/version.json 2/dev/null|jq .
{
version: 0.24.0
}
```
Thanks,
haosdent huang
Re: Review Request 37024: Add an endpoint that exposes component version.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37024/#review93861
---
Patch looks great!
Reviews applied: [37024]
All tests passed.
- Mesos ReviewBot
On Aug. 2, 2015, 10:16 a.m., haosdent huang wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37024/
---
(Updated Aug. 2, 2015, 10:16 a.m.)
Review request for mesos and Ben Mahler.
Bugs: MESOS-1841
https://issues.apache.org/jira/browse/MESOS-1841
Repository: mesos
Description
---
Add an endpoint that exposes component version.
Diffs
-
src/master/http.cpp 3772e39015a22655dcad00ad844dc5ddc90db43f
src/master/master.hpp ea18c4e0bb0743747401b9cd5ea14ae9b56ae3cc
src/master/master.cpp 351a3c2b5f551ad065682cea601d2436258e4544
src/slave/http.cpp b0fe5f520dfca156548ba8c436d42fc432223f3d
src/slave/slave.hpp 41d09497be313819a9c78361b8595f6f26dc8460
src/slave/slave.cpp 6b21db973dc95dd5eb2238eebe697db9dd063ef1
Diff: https://reviews.apache.org/r/37024/diff/
Testing
---
Manual test result:
```
$ curl http://localhost:5050/slave(1)/version.json 2/dev/null|jq .
{
version: 0.24.0
}
```
```
$ curl http://localhost:5050/master/version.json 2/dev/null|jq .
{
version: 0.24.0
}
```
Thanks,
haosdent huang
