> On Aug. 3, 2015, 6:43 p.m., Ben Mahler wrote:
> > src/master/http.cpp, lines 361-362
> > <https://reviews.apache.org/r/36987/diff/2/?file=1026437#file1026437line361>
> >
> >     Do we even want to keep this? Note that the 'observe' has never been 
> > used.
> >     
> >     I've found this to be a strange helper in the first place, how would we 
> > explain what this does in a comment?

> Note that the 'observe' has never been used.

What do you mean? It looks like `/observe` is an exposed endpoint that calls 
`observe`?
> I've found this to be a strange helper in the first place, how would we 
> explain what this does in a comment?

I should've accompanied it with a comment. My view of its functionality is 
something like:
```
Takes an HTTP request and the expected list of keys, tries to return the map of 
expected keys to their corresponding values.
It returns an Error if we failed to parse the request, if an expected key was 
not provided, or if the corresponding value is an empty string.
Otherwise, the resulting hashmap contains the mapping from expected keys to 
provided values.
```
I think it's a useful helper function for capturing the above functionality as 
well as to facilitate providing consistent error messages.
(e.g. for `/observe` we returned "Missing value for 'monitor'.", whereas for 
`/teardown` we returned "Missing 'frameworkId' query parameter")


- Michael


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


On July 31, 2015, 9:56 p.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36987/
> -----------------------------------------------------------
> 
> (Updated July 31, 2015, 9:56 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 3772e39015a22655dcad00ad844dc5ddc90db43f 
>   src/tests/repair_tests.cpp 4d513ed9e18ebf04a5e5aadb022b064479739e66 
> 
> Diff: https://reviews.apache.org/r/36987/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Reply via email to