> On Oct. 17, 2014, 7:33 a.m., Timothy Chen wrote:
> > src/files/files.cpp, line 59
> > <https://reviews.apache.org/r/26767/diff/1/?file=722434#file722434line59>
> >
> >     path asAbsolute as I remember is just sticking a / in front if it 
> > doesn't exist. It seems like it's only relevant in this query string path 
> > case? 
> >     I don't see how asAbsolute is ever useful anywhere else?

For files, they all take the query string and really should process it 
identically (Previously they weren't, files::attach("/foo/bar/"); 
Files::detach("/foo/bar/"); would not cause /foo/bar to be removed... They need 
to be identical and reverse.

As far as usefulness goes, it is fairly common when writing code that deals 
with paths passed in by users whether on the command line or through an HTTP 
API to normalize them so that they are always absolute paths (Particuarly when 
you change the current working directory of the program).  It's not something 
we use a lot of places, but it is a common operation on filesystem paths.

Most of the api in paths which I added is similar to functionality in python's 
os.path library


- Cody


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


On Oct. 23, 2014, 4:52 p.m., Cody Maloney wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26767/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2014, 4:52 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Timothy Chen.
> 
> 
> Bugs: MESOS-1878
>     https://issues.apache.org/jira/browse/MESOS-1878
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> The new path::join logic introduced changed how paths showed up in 
> files/files.hpp. Specifically, it made it so that paths more rigorously had / 
> didn't have a leading '/' (were absolute).
> 
> This caused some inconsistincies when trying to attach, detach, and resolve 
> paths, resulting in the files not being found, and therefore not showing up 
> in the webui.
> 
> The fix is primarily to standardize all the path manipulation routines (See 
> review 26766), testing the round trip path thoroughly, then updating files to 
> use the helpers to always clean / sanitize the incoming paths so the fit into 
> the same pattern.
> 
> This should resolve some lurking bugs around detach() as well. Unfortunately 
> I couldn't really add more tests there without changing the detach() return, 
> which was out of scope for this bugfix.
> 
> 
> Diffs
> -----
> 
>   src/files/files.hpp 818087b13cc787d0bd3186bb3e8a069751629bf9 
>   src/files/files.cpp 12e8f75aa7bd77d2e81d5d3a7a4d09dd915854aa 
>   src/tests/files_tests.cpp a696aa22d56b37ee70c6e64c81a849da6d436451 
> 
> Diff: https://reviews.apache.org/r/26767/diff/
> 
> 
> Testing
> -------
> 
> make distcheck
> 
> 
> Thanks,
> 
> Cody Maloney
> 
>

Reply via email to