You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Benjamin Hindman <be...@berkeley.edu> on 2013/10/22 01:59:00 UTC

Review Request 14815: Added an 'HDFS' abstraction.

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

Review request for mesos, Ben Mahler and Vinod Kone.


Repository: mesos-git


Description
-------

See summary.


Diffs
-----

  src/Makefile.am a2d82425f74dcaf3adf28ae8184fff53b3c34ceb 
  src/launcher/launcher.cpp bc39054141cd9451f3ff19e57f7b89a62b1de7fd 

Diff: https://reviews.apache.org/r/14815/diff/


Testing
-------

make check


Thanks,

Benjamin Hindman


Re: Review Request 14815: Added an 'HDFS' abstraction.

Posted by Benjamin Hindman <be...@berkeley.edu>.

> On Oct. 23, 2013, 6:53 p.m., Ben Mahler wrote:
> > Did you forget to add the hdfs.hpp file to your diff?

Fixed, thanks!


- Benjamin


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


On Oct. 23, 2013, 6:58 p.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14815/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2013, 6:58 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am a2d82425f74dcaf3adf28ae8184fff53b3c34ceb 
>   src/hdfs/hdfs.hpp PRE-CREATION 
>   src/launcher/launcher.cpp bc39054141cd9451f3ff19e57f7b89a62b1de7fd 
> 
> Diff: https://reviews.apache.org/r/14815/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>


Re: Review Request 14815: Added an 'HDFS' abstraction.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14815/#review27399
-----------------------------------------------------------


Did you forget to add the hdfs.hpp file to your diff?

- Ben Mahler


On Oct. 21, 2013, 11:59 p.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14815/
> -----------------------------------------------------------
> 
> (Updated Oct. 21, 2013, 11:59 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am a2d82425f74dcaf3adf28ae8184fff53b3c34ceb 
>   src/launcher/launcher.cpp bc39054141cd9451f3ff19e57f7b89a62b1de7fd 
> 
> Diff: https://reviews.apache.org/r/14815/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>


Re: Review Request 14815: Added an 'HDFS' abstraction.

Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14815/#review27549
-----------------------------------------------------------



src/hdfs/hdfs.hpp
<https://reviews.apache.org/r/14815/#comment53469>

    Make all HDFS methods const?


- Ian Downes


On Oct. 23, 2013, 6:58 p.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14815/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2013, 6:58 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am a2d82425f74dcaf3adf28ae8184fff53b3c34ceb 
>   src/hdfs/hdfs.hpp PRE-CREATION 
>   src/launcher/launcher.cpp bc39054141cd9451f3ff19e57f7b89a62b1de7fd 
> 
> Diff: https://reviews.apache.org/r/14815/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>


Re: Review Request 14815: Added an 'HDFS' abstraction.

Posted by Benjamin Hindman <be...@berkeley.edu>.

> On Oct. 25, 2013, 5:17 a.m., Vinod Kone wrote:
> >

Thanks for the review!


> On Oct. 25, 2013, 5:17 a.m., Vinod Kone wrote:
> > src/hdfs/hdfs.hpp, lines 55-56
> > <https://reviews.apache.org/r/14815/diff/2/?file=369926#file369926line55>
> >
> >     Doesn't look like you are using 'output'?

Thanks!


> On Oct. 25, 2013, 5:17 a.m., Vinod Kone wrote:
> > src/hdfs/hdfs.hpp, line 62
> > <https://reviews.apache.org/r/14815/diff/2/?file=369926#file369926line62>
> >
> >     Why do this instead of returning an error when exit status is non-zero like you do for other commands below?
> >     
> >     Also lets just have thing function return Try<Nothing> instead of Try<bool> ?

This is returning whether or not the path exists in HDFS so bool (true yes, false no) is needed as well as Try to determine if there was an error trying to get this information.


- Benjamin


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


On Oct. 23, 2013, 6:58 p.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14815/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2013, 6:58 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am a2d82425f74dcaf3adf28ae8184fff53b3c34ceb 
>   src/hdfs/hdfs.hpp PRE-CREATION 
>   src/launcher/launcher.cpp bc39054141cd9451f3ff19e57f7b89a62b1de7fd 
> 
> Diff: https://reviews.apache.org/r/14815/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>


Re: Review Request 14815: Added an 'HDFS' abstraction.

Posted by Vinod Kone <vi...@gmail.com>.

> On Oct. 25, 2013, 5:17 a.m., Vinod Kone wrote:
> > src/hdfs/hdfs.hpp, line 62
> > <https://reviews.apache.org/r/14815/diff/2/?file=369926#file369926line62>
> >
> >     Why do this instead of returning an error when exit status is non-zero like you do for other commands below?
> >     
> >     Also lets just have thing function return Try<Nothing> instead of Try<bool> ?
> 
> Benjamin Hindman wrote:
>     This is returning whether or not the path exists in HDFS so bool (true yes, false no) is needed as well as Try to determine if there was an error trying to get this information.

Oh you are right. I wasn't thinking straight.


- Vinod


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


On Oct. 23, 2013, 6:58 p.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14815/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2013, 6:58 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am a2d82425f74dcaf3adf28ae8184fff53b3c34ceb 
>   src/hdfs/hdfs.hpp PRE-CREATION 
>   src/launcher/launcher.cpp bc39054141cd9451f3ff19e57f7b89a62b1de7fd 
> 
> Diff: https://reviews.apache.org/r/14815/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>


Re: Review Request 14815: Added an 'HDFS' abstraction.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14815/#review27510
-----------------------------------------------------------

Ship it!



src/hdfs/hdfs.hpp
<https://reviews.apache.org/r/14815/#comment53446>

    Doesn't look like you are using 'output'?



src/hdfs/hdfs.hpp
<https://reviews.apache.org/r/14815/#comment53447>

    Why do this instead of returning an error when exit status is non-zero like you do for other commands below?
    
    Also lets just have thing function return Try<Nothing> instead of Try<bool> ?


- Vinod Kone


On Oct. 23, 2013, 6:58 p.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14815/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2013, 6:58 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am a2d82425f74dcaf3adf28ae8184fff53b3c34ceb 
>   src/hdfs/hdfs.hpp PRE-CREATION 
>   src/launcher/launcher.cpp bc39054141cd9451f3ff19e57f7b89a62b1de7fd 
> 
> Diff: https://reviews.apache.org/r/14815/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>


Re: Review Request 14815: Added an 'HDFS' abstraction.

Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14815/#review27546
-----------------------------------------------------------



src/hdfs/hdfs.hpp
<https://reviews.apache.org/r/14815/#comment53465>

    Should we include the standard ASF license?


- Ian Downes


On Oct. 23, 2013, 6:58 p.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14815/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2013, 6:58 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am a2d82425f74dcaf3adf28ae8184fff53b3c34ceb 
>   src/hdfs/hdfs.hpp PRE-CREATION 
>   src/launcher/launcher.cpp bc39054141cd9451f3ff19e57f7b89a62b1de7fd 
> 
> Diff: https://reviews.apache.org/r/14815/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>


Re: Review Request 14815: Added an 'HDFS' abstraction.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14815/
-----------------------------------------------------------

(Updated Oct. 23, 2013, 6:58 p.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


Changes
-------

Added src/hdfs/hdfs.hpp.


Repository: mesos-git


Description
-------

See summary.


Diffs (updated)
-----

  src/Makefile.am a2d82425f74dcaf3adf28ae8184fff53b3c34ceb 
  src/hdfs/hdfs.hpp PRE-CREATION 
  src/launcher/launcher.cpp bc39054141cd9451f3ff19e57f7b89a62b1de7fd 

Diff: https://reviews.apache.org/r/14815/diff/


Testing
-------

make check


Thanks,

Benjamin Hindman