You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Andrew Schwartzmeyer <an...@schwartzmeyer.com> on 2017/08/15 18:11:40 UTC
Re: 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/#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
>
>
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
>
>