mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Anand Mazumdar" <mazumdar.an...@gmail.com>
Subject Re: Review Request 41188: Providing JSON bindings to so that mesos modules can expose DiscoveryInfo protobuf messages to HTTP endpoints
Date Thu, 17 Dec 2015 00:45:39 GMT


> On Dec. 17, 2015, 12:12 a.m., Anand Mazumdar wrote:
> > src/tests/common/http_tests.cpp, lines 130-140
> > <https://reviews.apache.org/r/41188/diff/6/?file=1165163#file1165163line130>
> >
> >     Can you confirm that other objects that have labels that are exposed via `/state`
endpoint also have a nested `labels` mapping ?
> >     
> >     AFAICT, this is an artifact of us using the Protobuf to JSON converters when
we should have just used the `model(...)` functions.
> 
> Avinash sridharan wrote:
>     I am trying to understand this comment. Is it that applications don't expect nested
labels (for ports) when they read state.json? 
>     
>     In terms of exposing labels in state.json I can see from TEST_F(SlaveTest, TaskLabels)
(slave_tests.cpp) the labels are nested within tasks. Since, DiscoveryInfo also has labels
and Port has labels unless we nest them, the relationship would not be explicit if we do not
replicate the protobuf right?

Let me give you an example:

Here is how the Task Labels JSON returned from this test looks like:

```
"tasks": [
            {
              "executor_id": "default",
              "framework_id": "5452180f-fa83-4d25-802f-8a0c974dfd21-0000",
              "id": "1",
              "labels": [
                {
                  "key": "foo",
                  "value": "bar"
                },
                {
                  "key": "bar",
                  "value": "baz"
                },
                {
                  "key": "bar",
                  "value": "qux"
                }
              ],
              "name": "",
              "resources": {
                "cpus": 2,
                "disk": 1024,
                "mem": 1024,
                "ports": "[31000-32000]"
              },
```

If you notice, it does not have a "labels" field nested inside a "labels" field as we have
when we do automated protobuf to JSON conversions. Do we want to preserve this behavior ?


- Anand


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


On Dec. 15, 2015, 9:13 p.m., Avinash sridharan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41188/
> -----------------------------------------------------------
> 
> (Updated Dec. 15, 2015, 9:13 p.m.)
> 
> 
> Review request for mesos, Adam B and Anand Mazumdar.
> 
> 
> Bugs: MESOS-3962
>     https://issues.apache.org/jira/browse/MESOS-3962
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Providing JSON bindings to so that mesos modules can expose DiscoveryInfo protobuf messages
to HTTP endpoints
> 
> 
> Diffs
> -----
> 
>   src/slave/http.cpp d1b1158c0a80d72c32c9b28977043b6be2295239 
>   src/tests/common/http_tests.cpp 3aca5b0437a012664f58ff331cc7cf682d442699 
>   src/tests/slave_tests.cpp 4975bea8a7a701e0414426760692720f73dea7f5 
> 
> Diff: https://reviews.apache.org/r/41188/diff/
> 
> 
> Testing
> -------
> 
> make check.
> Added Unit tests to verify setting of DiscoveryInfo in state.json in slave. 
> Also added Unit test to test that DiscoveryInfo gets exposed in master when TaskInfo
protobuf is converted to JSON objects.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message