> On Oct. 13, 2015, 5 p.m., Michael Park wrote:
> > src/common/resources.cpp, line 361
> > <https://reviews.apache.org/r/39018/diff/10/?file=1096261#file1096261line361>
> >
> > I think it would be better to only support one way of doing things
> > here. I would opt for the array format. Is supporting both formats
> > explicitly a feature?
>
> Greg Mann wrote:
> No, it hasn't been explicitly called out as a feature. Why the desire to
> allow only one format? My thought was that any valid JSON string describing
> one or more Resource objects should return successfully, but I suppose there
> isn't too much need to accommodate a single resource object, since at the
> command line users will always be specifying multiple resources.
> Why the desire to allow only one format?
(1) Generally speaking, it's good to limit the number of ways to do the same
thing so that there's less to remember as the user
(2) In this specific case, it can actually be confusing/ambiguous.
That is, does this expect a single `Resource` object: `{ "name" : "cpus",
... }`, or
does it expect a `Credentials`-like object where we specify something like:
`{ "resources": { "name" : "cpus", ... }, { "name" : "disk", ... }, ... }`?
(I know now that it does the first one, but at first glance I actually
thought it does the second)
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39018/#review102443
-----------------------------------------------------------
On Oct. 13, 2015, 7:34 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39018/
> -----------------------------------------------------------
>
> (Updated Oct. 13, 2015, 7:34 p.m.)
>
>
> Review request for mesos, Adam B, Alexander Rukletsov, Jie Yu, and Michael
> Park.
>
>
> Bugs: MESOS-2467
> https://issues.apache.org/jira/browse/MESOS-2467
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This includes code changes necessary for JSON parsing of Resources.
> Documentation changes will be posted soon in another review
> (https://reviews.apache.org/r/39102/).
>
>
> Diffs
> -----
>
> include/mesos/resources.hpp 6c3a065945eb56dc88df9c977e5ca11d4cbcbf61
> include/mesos/v1/resources.hpp fe8925ac851b74d1b37919f00afc7ed816f47ea5
> src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1
> src/tests/resources_tests.cpp 6584fc6c39e6ffe9f8085576677dcc669f127697
> src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39
>
> Diff: https://reviews.apache.org/r/39018/diff/
>
>
> Testing
> -------
>
> New code was added to `ResourcesTest.ParsingFromJSON`, and two new tests were
> added: `ResourcesTest.ParsingFromJSONWithRoles` and
> `ResourcesTest.ParsingFromJSONError`. These attempt to test common error
> scenarios that might show up in user-supplied JSON.
>
> `make check` was used to confirm that all tests pass, with one notable
> failure (ResourcesTest.ParsingFromJSONError) related to a picojson issue
> tracked here: https://issues.apache.org/jira/browse/MESOS-3698
>
> The original resources parsing format is used throughout many other tests
> (check `src/tests/sorter_tests.cpp`, for example), so it's clear that the
> original parsing continues to work correctly.
>
>
> Thanks,
>
> Greg Mann
>
>
