Re: Review Request 60624: Enabled HDFS compilation and associated tests.

[Andrew Schwartzmeyer]

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


Ship it!




Ship It!

- Andrew Schwartzmeyer


On Dec. 7, 2017, 6:44 p.m., Jeff Coffler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60624/
> ---
> 
> (Updated Dec. 7, 2017, 6:44 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, Li 
> Li, and Michael Park.
> 
> 
> 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 35a602d2afb3a1e6ef76a0b0df2628ce5493dc81 
>   src/tests/CMakeLists.txt 92db731a81303f0d1d95dfe0b80a0a512e165445 
>   src/tests/hdfs_tests.cpp e7154c75e663d9a98bec48be42a59b65c96f9515 
> 
> 
> Diff: https://reviews.apache.org/r/60624/diff/7/
> 
> 
> Testing
> ---
> 
> See upstream
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>

Re: Review Request 60624: Enabled HDFS compilation and associated tests.

[Jeff Coffler]

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

(Updated Dec. 8, 2017, 2:44 a.m.)


Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, Li Li, 
and Michael Park.


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 35a602d2afb3a1e6ef76a0b0df2628ce5493dc81 
  src/tests/CMakeLists.txt 92db731a81303f0d1d95dfe0b80a0a512e165445 
  src/tests/hdfs_tests.cpp e7154c75e663d9a98bec48be42a59b65c96f9515 


Diff: https://reviews.apache.org/r/60624/diff/7/

Changes: https://reviews.apache.org/r/60624/diff/6-7/


Testing
---

See upstream


Thanks,

Jeff Coffler

Re: Review Request 60624: Enabled HDFS compilation and associated tests.

[Michael Park]

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


Fix it, then Ship it!





src/tests/hdfs_tests.cpp
Lines 55 (patched)


Let's mark as a `TODO`.



src/tests/hdfs_tests.cpp
Lines 71 (patched)


Let's mark these as `TODO`s here and below.


- Michael Park


On Nov. 6, 2017, 10:10 a.m., Jeff Coffler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60624/
> ---
> 
> (Updated Nov. 6, 2017, 10:10 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, Li 
> Li, and Michael Park.
> 
> 
> 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 219252f6f82b2d62d024b2ab876fa0ba2f5c8e6c 
>   src/tests/CMakeLists.txt 386e0473c93d0a993248c7818067071d0c761c76 
>   src/tests/hdfs_tests.cpp e7154c75e663d9a98bec48be42a59b65c96f9515 
> 
> 
> Diff: https://reviews.apache.org/r/60624/diff/6/
> 
> 
> Testing
> ---
> 
> See upstream
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>

Re: Review Request 60624: Enabled HDFS compilation and associated tests.

[Jeff Coffler]

---
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.

[Andrew Schwartzmeyer]

---
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.

[Jeff Coffler]

> On Oct. 17, 2017, 11:24 p.m., Andrew Schwartzmeyer wrote:
> > src/tests/hdfs_tests.cpp
> > Lines 55-57 (original), 55-60 (patched)
> > 
> >
> > 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.

[Jeff Coffler]

---
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.

[Andrew Schwartzmeyer]

---
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)


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.

[Jeff Coffler]

> On Oct. 13, 2017, 7:20 p.m., Andrew Schwartzmeyer wrote:
> > src/launcher/fetcher.cpp
> > Lines 267-272 (original), 267-275 (patched)
> > 
> >
> > 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.

[Jeff Coffler]

---
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.

[Andrew Schwartzmeyer]

---
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)


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.

[Jeff Coffler]

> 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)
> > 
> >
> > 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: Enabled HDFS compilation and associated tests.

[Jeff Coffler]

---
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