You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jeff Coffler <je...@taltos.com> on 2017/10/11 23:32:26 UTC
Re: Review Request 60624: Enabled HDFS compilation and associated
tests.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60624/
-----------------------------------------------------------
(Updated Oct. 11, 2017, 11:32 p.m.)
Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
Summary (updated)
-----------------
Enabled HDFS compilation and associated tests.
Bugs: MESOS-6705
https://issues.apache.org/jira/browse/MESOS-6705
Repository: mesos
Description (updated)
-------
Note that HDFS tests are disabled for Windows due to dependence on
'sh' shell.
Be aware: HDFS hasn't been tested, although it should work. We will
formally add support for HDFS at a later date, see MESOS-5460.
Diffs (updated)
-----
src/CMakeLists.txt 1a0dff3604c288d2af8ecc23f1ec73f5a350bc31
src/launcher/fetcher.cpp 5a9e93adbf2e4f0b8ff64a69e62fea5f6a0b8f2d
src/tests/CMakeLists.txt 386e0473c93d0a993248c7818067071d0c761c76
src/tests/hdfs_tests.cpp e7154c75e663d9a98bec48be42a59b65c96f9515
Diff: https://reviews.apache.org/r/60624/diff/3/
Changes: https://reviews.apache.org/r/60624/diff/2-3/
Testing
-------
See upstream
Thanks,
Jeff Coffler
Re: Review Request 60624: Enabled HDFS compilation and associated
tests.
Posted by Jeff Coffler <je...@taltos.com>.
> On Oct. 17, 2017, 11:24 p.m., Andrew Schwartzmeyer wrote:
> > src/tests/hdfs_tests.cpp
> > Lines 55-57 (original), 55-60 (patched)
> > <https://reviews.apache.org/r/60624/diff/4/?file=1858872#file1858872line55>
> >
> > Nit: this comment isn't great... we do know how to handle execution permissions on Windows. As in, the correct thing to do is not to mark it executable, as the concept simply doesn't map.
I disagree. Yes, it doesn't map, but there certainly isn't concensus in how to handle this.
We have both positive and negative tests dealing with the execution bit. I personally think the "right" solution is to have a run-time test that tells us if the platform supports that (I tend to prefer runtime tests to #ifdef's because they are often compiled out anyway, but will catch syntax errors that #ifdef's would miss). Then a test can "do the right thing" based on the platform and the test can pass everywhere (both on platforms with an execution bit and on platforms without an execution bit).
But, being an open source project, I need to write something up for this, distribute it to Mesos DEVs, and get buy-in. Until we have that, we don't know exactly how we're going to handle this.
This is on my list of things to do, and I will do it. But other DEVs may have other ideas on how to handle this properly.
- Jeff
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60624/#review188418
-----------------------------------------------------------
On Oct. 17, 2017, 1:18 a.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60624/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2017, 1:18 a.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
>
>
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Note that HDFS tests are disabled for Windows due to dependence on
> 'sh' shell.
>
> Be aware: HDFS hasn't been tested, although it should work. We will
> formally add support for HDFS at a later date, see MESOS-5460.
>
>
> Diffs
> -----
>
> src/CMakeLists.txt 1a0dff3604c288d2af8ecc23f1ec73f5a350bc31
> src/tests/CMakeLists.txt 386e0473c93d0a993248c7818067071d0c761c76
> src/tests/hdfs_tests.cpp e7154c75e663d9a98bec48be42a59b65c96f9515
>
>
> Diff: https://reviews.apache.org/r/60624/diff/4/
>
>
> Testing
> -------
>
> See upstream
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 60624: Enabled HDFS compilation and associated
tests.
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60624/#review188418
-----------------------------------------------------------
src/tests/hdfs_tests.cpp
Lines 55-57 (original), 55-60 (patched)
<https://reviews.apache.org/r/60624/#comment265418>
Nit: this comment isn't great... we do know how to handle execution permissions on Windows. As in, the correct thing to do is not to mark it executable, as the concept simply doesn't map.
- Andrew Schwartzmeyer
On Oct. 16, 2017, 6:18 p.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60624/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2017, 6:18 p.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
>
>
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Note that HDFS tests are disabled for Windows due to dependence on
> 'sh' shell.
>
> Be aware: HDFS hasn't been tested, although it should work. We will
> formally add support for HDFS at a later date, see MESOS-5460.
>
>
> Diffs
> -----
>
> src/CMakeLists.txt 1a0dff3604c288d2af8ecc23f1ec73f5a350bc31
> src/tests/CMakeLists.txt 386e0473c93d0a993248c7818067071d0c761c76
> src/tests/hdfs_tests.cpp e7154c75e663d9a98bec48be42a59b65c96f9515
>
>
> Diff: https://reviews.apache.org/r/60624/diff/4/
>
>
> Testing
> -------
>
> See upstream
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 60624: Enabled HDFS compilation and associated
tests.
Posted by Jeff Coffler <je...@taltos.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60624/
-----------------------------------------------------------
(Updated Oct. 19, 2017, 9:33 p.m.)
Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
Bugs: MESOS-6705
https://issues.apache.org/jira/browse/MESOS-6705
Repository: mesos
Description
-------
Note that HDFS tests are disabled for Windows due to dependence on
'sh' shell.
Be aware: HDFS hasn't been tested, although it should work. We will
formally add support for HDFS at a later date, see MESOS-5460.
Diffs (updated)
-----
src/CMakeLists.txt 219252f6f82b2d62d024b2ab876fa0ba2f5c8e6c
src/tests/CMakeLists.txt 386e0473c93d0a993248c7818067071d0c761c76
src/tests/hdfs_tests.cpp e7154c75e663d9a98bec48be42a59b65c96f9515
Diff: https://reviews.apache.org/r/60624/diff/6/
Changes: https://reviews.apache.org/r/60624/diff/5-6/
Testing
-------
See upstream
Thanks,
Jeff Coffler
Re: Review Request 60624: Enabled HDFS compilation and associated
tests.
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60624/#review188762
-----------------------------------------------------------
Ship it!
Ship It!
- Andrew Schwartzmeyer
On Oct. 19, 2017, 11:18 a.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60624/
> -----------------------------------------------------------
>
> (Updated Oct. 19, 2017, 11:18 a.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
>
>
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Note that HDFS tests are disabled for Windows due to dependence on
> 'sh' shell.
>
> Be aware: HDFS hasn't been tested, although it should work. We will
> formally add support for HDFS at a later date, see MESOS-5460.
>
>
> Diffs
> -----
>
> src/CMakeLists.txt 1a0dff3604c288d2af8ecc23f1ec73f5a350bc31
> src/tests/CMakeLists.txt 386e0473c93d0a993248c7818067071d0c761c76
> src/tests/hdfs_tests.cpp e7154c75e663d9a98bec48be42a59b65c96f9515
>
>
> Diff: https://reviews.apache.org/r/60624/diff/5/
>
>
> Testing
> -------
>
> See upstream
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 60624: Enabled HDFS compilation and associated
tests.
Posted by Jeff Coffler <je...@taltos.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60624/
-----------------------------------------------------------
(Updated Oct. 19, 2017, 6:18 p.m.)
Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
Bugs: MESOS-6705
https://issues.apache.org/jira/browse/MESOS-6705
Repository: mesos
Description
-------
Note that HDFS tests are disabled for Windows due to dependence on
'sh' shell.
Be aware: HDFS hasn't been tested, although it should work. We will
formally add support for HDFS at a later date, see MESOS-5460.
Diffs (updated)
-----
src/CMakeLists.txt 1a0dff3604c288d2af8ecc23f1ec73f5a350bc31
src/tests/CMakeLists.txt 386e0473c93d0a993248c7818067071d0c761c76
src/tests/hdfs_tests.cpp e7154c75e663d9a98bec48be42a59b65c96f9515
Diff: https://reviews.apache.org/r/60624/diff/5/
Changes: https://reviews.apache.org/r/60624/diff/4-5/
Testing
-------
See upstream
Thanks,
Jeff Coffler
Re: Review Request 60624: Enabled HDFS compilation and associated
tests.
Posted by Jeff Coffler <je...@taltos.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60624/
-----------------------------------------------------------
(Updated Oct. 17, 2017, 1:18 a.m.)
Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
Bugs: MESOS-6705
https://issues.apache.org/jira/browse/MESOS-6705
Repository: mesos
Description
-------
Note that HDFS tests are disabled for Windows due to dependence on
'sh' shell.
Be aware: HDFS hasn't been tested, although it should work. We will
formally add support for HDFS at a later date, see MESOS-5460.
Diffs (updated)
-----
src/CMakeLists.txt 1a0dff3604c288d2af8ecc23f1ec73f5a350bc31
src/tests/CMakeLists.txt 386e0473c93d0a993248c7818067071d0c761c76
src/tests/hdfs_tests.cpp e7154c75e663d9a98bec48be42a59b65c96f9515
Diff: https://reviews.apache.org/r/60624/diff/4/
Changes: https://reviews.apache.org/r/60624/diff/3-4/
Testing
-------
See upstream
Thanks,
Jeff Coffler
Re: Review Request 60624: Enabled HDFS compilation and associated
tests.
Posted by Jeff Coffler <je...@taltos.com>.
> On Oct. 13, 2017, 7:20 p.m., Andrew Schwartzmeyer wrote:
> > src/launcher/fetcher.cpp
> > Lines 267-272 (original), 267-275 (patched)
> > <https://reviews.apache.org/r/60624/diff/3/?file=1852890#file1852890line267>
> >
> > The changes to `fetcher.cpp` shouldn't be in this review, please move them to #60628.
This was originally done as part of the work to bring HDFS onboard, so it was checked in with that. But I'll merge with the other commit for the bulk of the fetcher work.
- Jeff
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60624/#review188002
-----------------------------------------------------------
On Oct. 17, 2017, 1:18 a.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60624/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2017, 1:18 a.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
>
>
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Note that HDFS tests are disabled for Windows due to dependence on
> 'sh' shell.
>
> Be aware: HDFS hasn't been tested, although it should work. We will
> formally add support for HDFS at a later date, see MESOS-5460.
>
>
> Diffs
> -----
>
> src/CMakeLists.txt 1a0dff3604c288d2af8ecc23f1ec73f5a350bc31
> src/tests/CMakeLists.txt 386e0473c93d0a993248c7818067071d0c761c76
> src/tests/hdfs_tests.cpp e7154c75e663d9a98bec48be42a59b65c96f9515
>
>
> Diff: https://reviews.apache.org/r/60624/diff/4/
>
>
> Testing
> -------
>
> See upstream
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 60624: Enabled HDFS compilation and associated
tests.
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60624/#review188002
-----------------------------------------------------------
This review should be combined with #60626.
src/launcher/fetcher.cpp
Lines 267-272 (original), 267-275 (patched)
<https://reviews.apache.org/r/60624/#comment265063>
The changes to `fetcher.cpp` shouldn't be in this review, please move them to #60628.
- Andrew Schwartzmeyer
On Oct. 11, 2017, 4:32 p.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60624/
> -----------------------------------------------------------
>
> (Updated Oct. 11, 2017, 4:32 p.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
>
>
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Note that HDFS tests are disabled for Windows due to dependence on
> 'sh' shell.
>
> Be aware: HDFS hasn't been tested, although it should work. We will
> formally add support for HDFS at a later date, see MESOS-5460.
>
>
> Diffs
> -----
>
> src/CMakeLists.txt 1a0dff3604c288d2af8ecc23f1ec73f5a350bc31
> src/launcher/fetcher.cpp 5a9e93adbf2e4f0b8ff64a69e62fea5f6a0b8f2d
> src/tests/CMakeLists.txt 386e0473c93d0a993248c7818067071d0c761c76
> src/tests/hdfs_tests.cpp e7154c75e663d9a98bec48be42a59b65c96f9515
>
>
> Diff: https://reviews.apache.org/r/60624/diff/3/
>
>
> Testing
> -------
>
> See upstream
>
>
> Thanks,
>
> Jeff Coffler
>
>