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/07/03 19:32:43 UTC
Review Request 60624: Enable HDFS compilation and associated tests.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60624/
-----------------------------------------------------------
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 tests are disabled for Windows due to dependence on 'sh'
shell.
Diffs
-----
src/CMakeLists.txt 51b67428f823454665db695ba70a0dc896a7595c
src/launcher/fetcher.cpp 42980f5a4a40b72f754156469e9fe60a952d1d87
src/tests/CMakeLists.txt 9c0acaf43f451dbc9ba5077529a36aa4cef40c34
src/tests/hdfs_tests.cpp e7154c75e663d9a98bec48be42a59b65c96f9515
Diff: https://reviews.apache.org/r/60624/diff/1/
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
>
>
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. 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. 8, 2017, 2:23 a.m., Andrew Schwartzmeyer wrote:
> > What is the reason that the HDFS files are now made to build on Windows, but (AFAICT) not actually enabled for support? Is there an immediate plan to support HDFS, and if not, why start building the files?
HDFS should work, although we haven't formally tested it. Added existing HDFS issue MESOS-5460; that still needs to be done.
This was added because Fetcher supports HDFS and, given that there wasn't anything specific there that would break Windows (other than the fact that HDFS wasn't installed), I thought it made sense to drag it in.
Note that some of the tests do run. Those that require 'sh' do not.
> On Oct. 8, 2017, 2:23 a.m., Andrew Schwartzmeyer wrote:
> > src/tests/hdfs_tests.cpp
> > Line 67 (original), 70 (patched)
> > <https://reviews.apache.org/r/60624/diff/2/?file=1847162#file1847162line70>
> >
> > All these disabled tests need an issue and note why they're disabled, as this patch is bringing the file in.
Added relevant comments.
- Jeff
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60624/#review187345
-----------------------------------------------------------
On Oct. 11, 2017, 11: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, 11: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
>
>
Re: Review Request 60624: Enable 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/#review187345
-----------------------------------------------------------
What is the reason that the HDFS files are now made to build on Windows, but (AFAICT) not actually enabled for support? Is there an immediate plan to support HDFS, and if not, why start building the files?
src/launcher/fetcher.cpp
Lines 267-268 (patched)
<https://reviews.apache.org/r/60624/#comment264275>
If so, this needs a TODO and an associated issue. The form should be:
// TODO(coffler): Fix Windows chmod handling, see MESOS-1234.
Or an explanation of why we can just ignore this on Windows (exe bit doesn't matter!).
src/launcher/fetcher.cpp
Lines 485-487 (patched)
<https://reviews.apache.org/r/60624/#comment264276>
Ditto style comment above.
src/launcher/fetcher.cpp
Lines 564-569 (original), 571-579 (patched)
<https://reviews.apache.org/r/60624/#comment264277>
Ditto style comment above.
For ownership, we probably need a separate issue tracking its implementation.
src/tests/hdfs_tests.cpp
Lines 55-57 (original), 55-60 (patched)
<https://reviews.apache.org/r/60624/#comment264278>
Ditto above on style / also exe bit doesn't map on Windows, so we can saefly ignore this logic.
src/tests/hdfs_tests.cpp
Line 67 (original), 70 (patched)
<https://reviews.apache.org/r/60624/#comment264279>
All these disabled tests need an issue and note why they're disabled, as this patch is bringing the file in.
- Andrew Schwartzmeyer
On Oct. 5, 2017, 9:04 p.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60624/
> -----------------------------------------------------------
>
> (Updated Oct. 5, 2017, 9:04 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 tests are disabled for Windows due to dependence on 'sh'
> shell.
>
>
> Diffs
> -----
>
> src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1
> src/launcher/fetcher.cpp 5a9e93adbf2e4f0b8ff64a69e62fea5f6a0b8f2d
> src/tests/CMakeLists.txt def233d13f627f58362fdc61ac3e9c104d65fe7e
> src/tests/hdfs_tests.cpp e7154c75e663d9a98bec48be42a59b65c96f9515
>
>
> Diff: https://reviews.apache.org/r/60624/diff/2/
>
>
> Testing
> -------
>
> See upstream
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 60624: Enable 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/#review187354
-----------------------------------------------------------
Summary and description need to follow Mesos standards (past-tense, full-sentence summary, with description in the body).
- Andrew Schwartzmeyer
On Oct. 5, 2017, 9:04 p.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60624/
> -----------------------------------------------------------
>
> (Updated Oct. 5, 2017, 9:04 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 tests are disabled for Windows due to dependence on 'sh'
> shell.
>
>
> Diffs
> -----
>
> src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1
> src/launcher/fetcher.cpp 5a9e93adbf2e4f0b8ff64a69e62fea5f6a0b8f2d
> src/tests/CMakeLists.txt def233d13f627f58362fdc61ac3e9c104d65fe7e
> src/tests/hdfs_tests.cpp e7154c75e663d9a98bec48be42a59b65c96f9515
>
>
> Diff: https://reviews.apache.org/r/60624/diff/2/
>
>
> Testing
> -------
>
> See upstream
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 60624: Enable 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. 6, 2017, 4:04 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 tests are disabled for Windows due to dependence on 'sh'
shell.
Diffs (updated)
-----
src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1
src/launcher/fetcher.cpp 5a9e93adbf2e4f0b8ff64a69e62fea5f6a0b8f2d
src/tests/CMakeLists.txt def233d13f627f58362fdc61ac3e9c104d65fe7e
src/tests/hdfs_tests.cpp e7154c75e663d9a98bec48be42a59b65c96f9515
Diff: https://reviews.apache.org/r/60624/diff/2/
Changes: https://reviews.apache.org/r/60624/diff/1-2/
Testing
-------
See upstream
Thanks,
Jeff Coffler
Re: Review Request 60624: Enable HDFS compilation and associated
tests.
Posted by Jeff Coffler <je...@taltos.com>.
> On Aug. 15, 2017, 6:11 p.m., Andrew Schwartzmeyer wrote:
> > src/launcher/fetcher.cpp
> > Lines 267 (patched)
> > <https://reviews.apache.org/r/60624/diff/1/?file=1768704#file1768704line267>
> >
> > This one is straight-forward. On Windows, there is no "executable permissions" so we can simply ignore this logic. With a note of course as to why.
This needs to be generally revisited, and is on the list. This is a much bigger change, I want to handle it all at once.
> On Aug. 15, 2017, 6:11 p.m., Andrew Schwartzmeyer wrote:
> > src/launcher/fetcher.cpp
> > Lines 487 (patched)
> > <https://reviews.apache.org/r/60624/diff/1/?file=1768704#file1768704line487>
> >
> > I'm not suggesting to do this _right now_. But for `chown`, Windows has a mappable concept of taking (probably recursive) ownership of the directory as the specified user. So we can handle this, we'll just need to implement it.
Permissions is an issue throughout Mesos today, and should be looked at at that time.
> On Aug. 15, 2017, 6:11 p.m., Andrew Schwartzmeyer wrote:
> > src/launcher/fetcher.cpp
> > Lines 564 (patched)
> > <https://reviews.apache.org/r/60624/diff/1/?file=1768704#file1768704line564>
> >
> > I think we can do the same thing as `os::su` with Windows user impersonation. It's a mappable concept.
It's only mappable when we get the permissions thing worked out, and we can run on something other than an admistrative account.
> On Aug. 15, 2017, 6:11 p.m., Andrew Schwartzmeyer wrote:
> > src/tests/hdfs_tests.cpp
> > Lines 56 (patched)
> > <https://reviews.apache.org/r/60624/diff/1/?file=1768706#file1768706line56>
> >
> > Like above, we can safely ignore setting execution permission on Windows. My reasoning is that the concept maps to nothing. So doing nothing is appropriate I think.
Again, this sort of thing is in many places (including other modules), and I'd like to fix that issue all at once.
- Jeff
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60624/#review182972
-----------------------------------------------------------
On July 3, 2017, 7:32 p.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60624/
> -----------------------------------------------------------
>
> (Updated July 3, 2017, 7: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 tests are disabled for Windows due to dependence on 'sh'
> shell.
>
>
> Diffs
> -----
>
> src/CMakeLists.txt 51b67428f823454665db695ba70a0dc896a7595c
> src/launcher/fetcher.cpp 42980f5a4a40b72f754156469e9fe60a952d1d87
> src/tests/CMakeLists.txt 9c0acaf43f451dbc9ba5077529a36aa4cef40c34
> src/tests/hdfs_tests.cpp e7154c75e663d9a98bec48be42a59b65c96f9515
>
>
> Diff: https://reviews.apache.org/r/60624/diff/1/
>
>
> Testing
> -------
>
> See upstream
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 60624: Enable 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/#review182972
-----------------------------------------------------------
src/CMakeLists.txt
Line 513 (original), 514 (patched)
<https://reviews.apache.org/r/60624/#comment258942>
Uh.
src/launcher/fetcher.cpp
Lines 267 (patched)
<https://reviews.apache.org/r/60624/#comment258943>
This one is straight-forward. On Windows, there is no "executable permissions" so we can simply ignore this logic. With a note of course as to why.
src/launcher/fetcher.cpp
Lines 487 (patched)
<https://reviews.apache.org/r/60624/#comment258946>
I'm not suggesting to do this _right now_. But for `chown`, Windows has a mappable concept of taking (probably recursive) ownership of the directory as the specified user. So we can handle this, we'll just need to implement it.
src/launcher/fetcher.cpp
Lines 564 (patched)
<https://reviews.apache.org/r/60624/#comment258947>
I think we can do the same thing as `os::su` with Windows user impersonation. It's a mappable concept.
src/tests/hdfs_tests.cpp
Lines 56 (patched)
<https://reviews.apache.org/r/60624/#comment258949>
Like above, we can safely ignore setting execution permission on Windows. My reasoning is that the concept maps to nothing. So doing nothing is appropriate I think.
- Andrew Schwartzmeyer
On July 3, 2017, 12:32 p.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60624/
> -----------------------------------------------------------
>
> (Updated July 3, 2017, 12: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 tests are disabled for Windows due to dependence on 'sh'
> shell.
>
>
> Diffs
> -----
>
> src/CMakeLists.txt 51b67428f823454665db695ba70a0dc896a7595c
> src/launcher/fetcher.cpp 42980f5a4a40b72f754156469e9fe60a952d1d87
> src/tests/CMakeLists.txt 9c0acaf43f451dbc9ba5077529a36aa4cef40c34
> src/tests/hdfs_tests.cpp e7154c75e663d9a98bec48be42a59b65c96f9515
>
>
> Diff: https://reviews.apache.org/r/60624/diff/1/
>
>
> Testing
> -------
>
> See upstream
>
>
> Thanks,
>
> Jeff Coffler
>
>