> 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? > > Anand Mazumdar wrote: > 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 ?
I get your point . You are right this is an artifact of using the JSON::protobuf compred to explicit model functions . Is this going to be an issue? Since the dependency is reflected in the protobuf definition. One reason I can of think of keeping it as is, is that the protobuf is a contract between the framework and service discovery, so they might actually expect this. - Avinash ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41188/#review110804 ----------------------------------------------------------- On Dec. 17, 2015, 3:10 a.m., Avinash sridharan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41188/ > ----------------------------------------------------------- > > (Updated Dec. 17, 2015, 3:10 a.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 > >
