Re: Review Request 39968: Enable endpoint include nested paths

2016-02-10 Thread Ben Mahler 

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




3rdparty/libprocess/src/help.cpp (lines 120 - 131)


Did you test the help endpoints when you made this change? This code looks 
incorrect for `tokens.size() == 3`, how about we update tokenize to take the 
maximum number of tokens, like we did for strings::split?

```
  // Path format of the url: /help/[/]
  // Note that  may contain slashes.
  vector tokens = strings::tokenize(request.url.path, "/", 3);

  Option id = None();
  Option name = None();

  if (tokens.size() > 1) {
id = tokens[1];
  }
  
  if (tokens.size() > 2) {
name = tokens[2];
  }
```


- Ben Mahler


On Nov. 5, 2015, 7:25 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39968/
> ---
> 
> (Updated Nov. 5, 2015, 7:25 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3833
> https://issues.apache.org/jira/browse/MESOS-3833
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enable endpoint include nested paths
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/help.cpp 3f82c0fb41a0d9388d88dbecc9289d705c2a343a 
> 
> Diff: https://reviews.apache.org/r/39968/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 39968: Enable endpoint include nested paths

2016-02-09 Thread haosdent huang 

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




3rdparty/libprocess/src/help.cpp (line 122)


I think could use
```
strings::split(strings::trim(request.url.path, strings::ANY, "/"), "/", 3);
```
to handle this part.
`tokens[1]` is always the id while `tokens[2]` is the name when 
`tokens.size() >= 3`.


- haosdent huang


On Nov. 5, 2015, 7:25 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39968/
> ---
> 
> (Updated Nov. 5, 2015, 7:25 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3833
> https://issues.apache.org/jira/browse/MESOS-3833
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enable endpoint include nested paths
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/help.cpp 3f82c0fb41a0d9388d88dbecc9289d705c2a343a 
> 
> Diff: https://reviews.apache.org/r/39968/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 39968: Enable endpoint include nested paths

2015-11-05 Thread Mesos ReviewBot 

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


Patch looks great!

Reviews applied: [39968]

All tests passed.

- Mesos ReviewBot


On Nov. 5, 2015, 7:25 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39968/
> ---
> 
> (Updated Nov. 5, 2015, 7:25 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3833
> https://issues.apache.org/jira/browse/MESOS-3833
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enable endpoint include nested paths
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/help.cpp 3f82c0fb41a0d9388d88dbecc9289d705c2a343a 
> 
> Diff: https://reviews.apache.org/r/39968/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 39968: Enable endpoint include nested paths

2015-11-04 Thread Guangya Liu 

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


I'm not sure if this is a good solution or not, @Bmahler, please show your 
comments if any and I can update this RR based on your comments. Thanks!

- Guangya Liu


On 十一月 5, 2015, 7:25 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39968/
> ---
> 
> (Updated 十一月 5, 2015, 7:25 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3833
> https://issues.apache.org/jira/browse/MESOS-3833
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enable endpoint include nested paths
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/help.cpp 3f82c0fb41a0d9388d88dbecc9289d705c2a343a 
> 
> Diff: https://reviews.apache.org/r/39968/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>