You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by John Kordich via Review Board <no...@reviews.apache.org> on 2018/03/09 20:17:16 UTC
Re: Review Request 66002: Fixed the HTTP API path variables on
Windows.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66002/
-----------------------------------------------------------
(Updated March 9, 2018, 8:17 p.m.)
Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.
Changes
-------
Expanded commit message
Summary (updated)
-----------------
Fixed the HTTP API path variables on Windows.
Repository: mesos
Description (updated)
-------
Many mesos frameworks assume that path separators are forward slashes,
like dcos-ui and marathon-ui. dcos-cli also assumes this. Previous to
this patch, if a forward slash was given in the path variable to an HTTP
API function call to the mesos agent on a Windows system, the Windows
API would fail at recognizing the path, because the Windows API accepts
only backslashes as path separators. To remedy this issue, we now
convert all forward slashes passed as a path to the HTTP API to an agent
to back slashes on Windows agents by using the path::from_uri function.
Diffs (updated)
-----
src/files/files.cpp 5f92d2af722c3201200fa1d6a75dc0b87fdc6078
src/tests/files_tests.cpp c703cae03345112715aeab83cb0a74abe3e12469
Diff: https://reviews.apache.org/r/66002/diff/2/
Changes: https://reviews.apache.org/r/66002/diff/1-2/
Testing
-------
Tested on both Windows and Linux, all tests pass, including the two newly enabled tests on Windows.
Thanks,
John Kordich
Re: Review Request 66002: Fixed the HTTP API path variables on
Windows.
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66002/#review199204
-----------------------------------------------------------
src/files/files.cpp
Lines 314-328 (original), 314-329 (patched)
<https://reviews.apache.org/r/66002/#comment279505>
const
src/files/files.cpp
Line 350 (original), 352 (patched)
<https://reviews.apache.org/r/66002/#comment279507>
const
src/files/files.cpp
Line 384 (original), 387 (patched)
<https://reviews.apache.org/r/66002/#comment279508>
Ha, this ought to be const too even though you didn't add the variable.
I sure wish C++ default to const like Rust :(
src/files/files.cpp
Line 453 (original), 458 (patched)
<https://reviews.apache.org/r/66002/#comment279509>
const
src/files/files.cpp
Line 628 (original), 634 (patched)
<https://reviews.apache.org/r/66002/#comment279510>
const
src/files/files.cpp
Line 769 (original), 776 (patched)
<https://reviews.apache.org/r/66002/#comment279511>
const
src/tests/files_tests.cpp
Lines 256-257 (original), 256-257 (patched)
<https://reviews.apache.org/r/66002/#comment279513>
With the above fixes, `files.attach` now accepts URI like paths, right? Or did we skip `attach`? Should we? I don't know.
src/tests/files_tests.cpp
Lines 336-345 (original), 336-349 (patched)
<https://reviews.apache.org/r/66002/#comment279515>
Ew. Can we test these without `stat`? I don't want to add to [MESOS-8275](https://issues.apache.org/jira/browse/MESOS-8275)
- Andrew Schwartzmeyer
On March 9, 2018, 12:17 p.m., John Kordich wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66002/
> -----------------------------------------------------------
>
> (Updated March 9, 2018, 12:17 p.m.)
>
>
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Many mesos frameworks assume that path separators are forward slashes,
> like dcos-ui and marathon-ui. dcos-cli also assumes this. Previous to
> this patch, if a forward slash was given in the path variable to an HTTP
> API function call to the mesos agent on a Windows system, the Windows
> API would fail at recognizing the path, because the Windows API accepts
> only backslashes as path separators. To remedy this issue, we now
> convert all forward slashes passed as a path to the HTTP API to an agent
> to back slashes on Windows agents by using the path::from_uri function.
>
>
> Diffs
> -----
>
> src/files/files.cpp 5f92d2af722c3201200fa1d6a75dc0b87fdc6078
> src/tests/files_tests.cpp c703cae03345112715aeab83cb0a74abe3e12469
>
>
> Diff: https://reviews.apache.org/r/66002/diff/2/
>
>
> Testing
> -------
>
> Tested on both Windows and Linux, all tests pass, including the two newly enabled tests on Windows.
>
>
> Thanks,
>
> John Kordich
>
>
Re: Review Request 66002: Fixed the HTTP API path variables on
Windows.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66002/#review198967
-----------------------------------------------------------
FAIL: Some of the unit tests failed. Please check the relevant logs.
Reviews applied: `['66002']`
Failed command: `Start-MesosCITesting`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66002
Relevant logs:
- [mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66002/logs/mesos-tests-stdout.log):
```
[ OK ] Endpoint/SlaveEndpointTest.NoAuthorizer/2 (128 ms)
[----------] 9 tests from Endpoint/SlaveEndpointTest (1223 ms total)
[----------] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[ OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (43 ms)
[ RUN ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[ OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (46 ms)
[----------] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (92 ms total)
[----------] 1 test from IsolationFlag/CpuIsolatorTest
[ RUN ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0
[ OK ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0 (2495 ms)
[----------] 1 test from IsolationFlag/CpuIsolatorTest (2520 ms total)
[----------] 1 test from IsolationFlag/MemoryIsolatorTest
[ RUN ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0
[ OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (2478 ms)
[----------] 1 test from IsolationFlag/MemoryIsolatorTest (2502 ms total)
[----------] Global test environment tear-down
[==========] 918 tests from 91 test cases ran. (474724 ms total)
[ PASSED ] 917 tests.
[ FAILED ] 1 test, listed below:
[ FAILED ] CommandExecutorCheckTest.CommandCheckTimeout
1 FAILED TEST
YOU HAVE 208 DISABLED TESTS
```
- [mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66002/logs/mesos-tests-stderr.log):
```
I0309 20:33:19.885869 2232 slave.cpp:3878] Shutting down framework 7d4e00f9-21e5-42e2-8624-0637b93a0e3b-0000
I0309 20:33:19.885869 7700 master.cpp:10210] Updating the state of task ca531887-b68c-4bce-96ac-514ec6d35811 of framework 7d4e00f9-21e5-42e2-8624-0637b93a0e3b-0000 (latest state: TASK_KILLED, status update state: TASK_KILLED)
I0309 20:33:19.885869 2232 slave.cpp:6571] Shutting down executor 'ca531887-b68c-4bce-96ac-514ec6d35811' of framework 7d4e00f9-21e5-42e2-8624-0637b93a0e3b-0000 at executor(1)@10.3.1.11:61477
I0309 20:33:19.888870 2232 slave.cpp:924] Agent terminating
W0309 20:33:19.888870 2232 slave.cpp:3874] Ignoring shutdown framework 7d4e00f9-21e5-42e2-8624-0637b93a0e3b-0000 because it is terminating
I0309 20:33:19.889871 7700 master.cpp:10309] Removing task ca531887-b68c-4bce-96ac-514ec6d35811 with resources cpus(allocated: *):4; mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: *):[31000-32000] of framework 7d4e00f9-21e5-42e2-8624-0637b93a0e3b-0000 on agent 7d4e00f9-21e5-42e2-8624-0637b93a0e3b-S0 at slave(398)@10.3.1.11:61455 (build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.I0309 20:33:19.178894 3528 exec.cpp:162] Version: 1.6.0
I0309 20:33:19.208870 9684 exec.cpp:236] Executor registered on agent 7d4e00f9-21e5-42e2-8624-0637b93a0e3b-S0
I0309 20:33:19.212896 9144 executor.cpp:176] Received SUBSCRIBED event
I0309 20:33:19.218895 9144 executor.cpp:180] Subscribed executor on build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I0309 20:33:19.218895 9144 executor.cpp:176] Received LAUNCH event
I0309 20:33:19.223894 9144 executor.cpp:648] Starting task ca531887-b68c-4bce-96ac-514ec6d35811
I0309 20:33:19.307874 9144 executor.cpp:483] Running 'D:\DCOS\mesos\src\mesos-containerizer.exe launch <POSSIBLY-SENSITIVE-DATA>'
I0309 20:33:19.848868 9144 executor.cpp:661] Forked command at 3252
I0309 20:33:19.889871 5412 exec.cpp:445] Executor asked to shutdown
I0309 20:33:19.890872 9144 executor.cpp:176] Received SHUTDOWN event
I0309 20:33:19.890872 9144 executor.cpp:758] Shutting down
I0309 20:33:19.890872 9144 executor.cpp:868] Sending SIGTERM to process tree at pid 3net)
I0309 20:33:19.893869 10732 containerizer.cpp:2338] Destroying container 017d2658-dafb-43ee-8e1b-0439b6f41b3f in RUNNING state
I0309 20:33:19.893869 10732 containerizer.cpp:2952] Transitioning the state of container 017d2658-dafb-43ee-8e1b-0439b6f41b3f from RUNNING to DESTROYING
I0309 20:33:19.894868 10732 launcher.cpp:156] Asked to destroy container 017d2658-dafb-43ee-8e1b-0439b6f41b3f
I0309 20:33:19.894868 7700 master.cpp:1306] Agent 7d4e00f9-21e5-42e2-8624-0637b93a0e3b-S0 at slave(398)@10.3.1.11:61455 (build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net) disconnected
I0309 20:33:19.894868 7700 master.cpp:3276] Disconnecting agent 7d4e00f9-21e5-42e2-8624-0637b93a0e3b-S0 at slave(398)@10.3.1.11:61455 (build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0309 20:33:19.895920 7700 master.cpp:3295] Deactivating agent 7d4e00f9-21e5-42e2-8624-0637b93a0e3b-S0 at slave(398)@10.3.1.11:61455 (build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0309 20:33:19.895920 6616 hierarchical.cpp:344] Removed framework 7d4e00f9-21e5-42e2-8624-0637b93a0e3b-0000
I0309 20:33:19.897879 6616 hierarchical.cpp:766] Agent 7d4e00f9-21e5-42e2-8624-0637b93a0e3b-S0 deactivated
I0309 20:33:19.962121 2232 containerizer.cpp:2791] Container 017d2658-dafb-43ee-8e1b-0439b6f41b3f has exited
I0309 20:33:19.994122 7008 master.cpp:1149] Master terminating
I0309 20:33:19.996094 10732 hierarchical.cpp:609] Removed agent 7d4e00f9-21e5-42e2-8624-0637b93a0e3b-S0
I0309 20:33:20.790071 6140 process.cpp:929] Stopped the socket accept loop
```
- Mesos Reviewbot Windows
On March 9, 2018, 8:17 p.m., John Kordich wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66002/
> -----------------------------------------------------------
>
> (Updated March 9, 2018, 8:17 p.m.)
>
>
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Many mesos frameworks assume that path separators are forward slashes,
> like dcos-ui and marathon-ui. dcos-cli also assumes this. Previous to
> this patch, if a forward slash was given in the path variable to an HTTP
> API function call to the mesos agent on a Windows system, the Windows
> API would fail at recognizing the path, because the Windows API accepts
> only backslashes as path separators. To remedy this issue, we now
> convert all forward slashes passed as a path to the HTTP API to an agent
> to back slashes on Windows agents by using the path::from_uri function.
>
>
> Diffs
> -----
>
> src/files/files.cpp 5f92d2af722c3201200fa1d6a75dc0b87fdc6078
> src/tests/files_tests.cpp c703cae03345112715aeab83cb0a74abe3e12469
>
>
> Diff: https://reviews.apache.org/r/66002/diff/2/
>
>
> Testing
> -------
>
> Tested on both Windows and Linux, all tests pass, including the two newly enabled tests on Windows.
>
>
> Thanks,
>
> John Kordich
>
>
Re: Review Request 66002: Fixed the HTTP API path variables on
Windows.
Posted by Jeff Coffler <je...@taltos.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66002/#review199213
-----------------------------------------------------------
Ship it!
Ship It!
- Jeff Coffler
On March 9, 2018, 8:17 p.m., John Kordich wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66002/
> -----------------------------------------------------------
>
> (Updated March 9, 2018, 8:17 p.m.)
>
>
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Many mesos frameworks assume that path separators are forward slashes,
> like dcos-ui and marathon-ui. dcos-cli also assumes this. Previous to
> this patch, if a forward slash was given in the path variable to an HTTP
> API function call to the mesos agent on a Windows system, the Windows
> API would fail at recognizing the path, because the Windows API accepts
> only backslashes as path separators. To remedy this issue, we now
> convert all forward slashes passed as a path to the HTTP API to an agent
> to back slashes on Windows agents by using the path::from_uri function.
>
>
> Diffs
> -----
>
> src/files/files.cpp 5f92d2af722c3201200fa1d6a75dc0b87fdc6078
> src/tests/files_tests.cpp c703cae03345112715aeab83cb0a74abe3e12469
>
>
> Diff: https://reviews.apache.org/r/66002/diff/2/
>
>
> Testing
> -------
>
> Tested on both Windows and Linux, all tests pass, including the two newly enabled tests on Windows.
>
>
> Thanks,
>
> John Kordich
>
>
Re: Review Request 66002: Fixed the HTTP API path variables on
Windows.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66002/#review198983
-----------------------------------------------------------
Patch looks great!
Reviews applied: [66002]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On March 9, 2018, 8:17 p.m., John Kordich wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66002/
> -----------------------------------------------------------
>
> (Updated March 9, 2018, 8:17 p.m.)
>
>
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Many mesos frameworks assume that path separators are forward slashes,
> like dcos-ui and marathon-ui. dcos-cli also assumes this. Previous to
> this patch, if a forward slash was given in the path variable to an HTTP
> API function call to the mesos agent on a Windows system, the Windows
> API would fail at recognizing the path, because the Windows API accepts
> only backslashes as path separators. To remedy this issue, we now
> convert all forward slashes passed as a path to the HTTP API to an agent
> to back slashes on Windows agents by using the path::from_uri function.
>
>
> Diffs
> -----
>
> src/files/files.cpp 5f92d2af722c3201200fa1d6a75dc0b87fdc6078
> src/tests/files_tests.cpp c703cae03345112715aeab83cb0a74abe3e12469
>
>
> Diff: https://reviews.apache.org/r/66002/diff/2/
>
>
> Testing
> -------
>
> Tested on both Windows and Linux, all tests pass, including the two newly enabled tests on Windows.
>
>
> Thanks,
>
> John Kordich
>
>
Re: Review Request 66002: Fixed the HTTP API path variables on
Windows.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66002/#review199483
-----------------------------------------------------------
Patch looks great!
Reviews applied: [66002]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On March 19, 2018, 11:20 a.m., John Kordich wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66002/
> -----------------------------------------------------------
>
> (Updated March 19, 2018, 11:20 a.m.)
>
>
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Many mesos frameworks assume that path separators are forward slashes,
> like dcos-ui and marathon-ui. dcos-cli also assumes this. Previous to
> this patch, if a forward slash was given in the path variable to an HTTP
> API function call to the mesos agent on a Windows system, the Windows
> API would fail at recognizing the path, because the Windows API accepts
> only backslashes as path separators. To remedy this issue, we will now
> convert all forward slashes passed as a path to the HTTP API to back
> slashes on Windows agents by using the path::from_uri function.
>
>
> Diffs
> -----
>
> src/files/files.cpp 5f92d2af722c3201200fa1d6a75dc0b87fdc6078
> src/tests/files_tests.cpp c703cae03345112715aeab83cb0a74abe3e12469
>
>
> Diff: https://reviews.apache.org/r/66002/diff/3/
>
>
> Testing
> -------
>
> Tested on both Windows and Linux, all tests pass, including the two newly enabled tests on Windows.
>
>
> Thanks,
>
> John Kordich
>
>
Re: Review Request 66002: Fixed the HTTP API path variables on
Windows.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66002/#review199478
-----------------------------------------------------------
PASS: Mesos patch 66002 was successfully built and tested.
Reviews applied: `['66002']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66002
- Mesos Reviewbot Windows
On March 19, 2018, 6:20 p.m., John Kordich wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66002/
> -----------------------------------------------------------
>
> (Updated March 19, 2018, 6:20 p.m.)
>
>
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Many mesos frameworks assume that path separators are forward slashes,
> like dcos-ui and marathon-ui. dcos-cli also assumes this. Previous to
> this patch, if a forward slash was given in the path variable to an HTTP
> API function call to the mesos agent on a Windows system, the Windows
> API would fail at recognizing the path, because the Windows API accepts
> only backslashes as path separators. To remedy this issue, we will now
> convert all forward slashes passed as a path to the HTTP API to back
> slashes on Windows agents by using the path::from_uri function.
>
>
> Diffs
> -----
>
> src/files/files.cpp 5f92d2af722c3201200fa1d6a75dc0b87fdc6078
> src/tests/files_tests.cpp c703cae03345112715aeab83cb0a74abe3e12469
>
>
> Diff: https://reviews.apache.org/r/66002/diff/3/
>
>
> Testing
> -------
>
> Tested on both Windows and Linux, all tests pass, including the two newly enabled tests on Windows.
>
>
> Thanks,
>
> John Kordich
>
>
Re: Review Request 66002: Fixed the HTTP API path variables on
Windows.
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66002/#review199511
-----------------------------------------------------------
Ship it!
Ship It!
- Andrew Schwartzmeyer
On March 19, 2018, 11:20 a.m., John Kordich wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66002/
> -----------------------------------------------------------
>
> (Updated March 19, 2018, 11:20 a.m.)
>
>
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Many mesos frameworks assume that path separators are forward slashes,
> like dcos-ui and marathon-ui. dcos-cli also assumes this. Previous to
> this patch, if a forward slash was given in the path variable to an HTTP
> API function call to the mesos agent on a Windows system, the Windows
> API would fail at recognizing the path, because the Windows API accepts
> only backslashes as path separators. To remedy this issue, we will now
> convert all forward slashes passed as a path to the HTTP API to back
> slashes on Windows agents by using the path::from_uri function.
>
>
> Diffs
> -----
>
> src/files/files.cpp 5f92d2af722c3201200fa1d6a75dc0b87fdc6078
> src/tests/files_tests.cpp c703cae03345112715aeab83cb0a74abe3e12469
>
>
> Diff: https://reviews.apache.org/r/66002/diff/3/
>
>
> Testing
> -------
>
> Tested on both Windows and Linux, all tests pass, including the two newly enabled tests on Windows.
>
>
> Thanks,
>
> John Kordich
>
>
Re: Review Request 66002: Fixed the HTTP API path variables on
Windows.
Posted by John Kordich via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66002/
-----------------------------------------------------------
(Updated March 19, 2018, 6:20 p.m.)
Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.
Changes
-------
Addressed comments
Repository: mesos
Description (updated)
-------
Many mesos frameworks assume that path separators are forward slashes,
like dcos-ui and marathon-ui. dcos-cli also assumes this. Previous to
this patch, if a forward slash was given in the path variable to an HTTP
API function call to the mesos agent on a Windows system, the Windows
API would fail at recognizing the path, because the Windows API accepts
only backslashes as path separators. To remedy this issue, we will now
convert all forward slashes passed as a path to the HTTP API to back
slashes on Windows agents by using the path::from_uri function.
Diffs (updated)
-----
src/files/files.cpp 5f92d2af722c3201200fa1d6a75dc0b87fdc6078
src/tests/files_tests.cpp c703cae03345112715aeab83cb0a74abe3e12469
Diff: https://reviews.apache.org/r/66002/diff/3/
Changes: https://reviews.apache.org/r/66002/diff/2-3/
Testing
-------
Tested on both Windows and Linux, all tests pass, including the two newly enabled tests on Windows.
Thanks,
John Kordich
Re: Review Request 66002: Fixed the HTTP API path variables on
Windows.
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66002/#review199206
-----------------------------------------------------------
Ship it!
I think this approach is reasonable. `files` is the "public" API where we're getting (essentially) user input (i.e. things from frameworks and other external code beyond our control). This API had previously always expected `/` as the path separator, I think this is the correct place to "sanitize user input" and convert to the OS-specific path separator. This way, our OS layer (the file functions in stout) are left OS-specific, but the public API will work as expected.
- Andrew Schwartzmeyer
On March 9, 2018, 12:17 p.m., John Kordich wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66002/
> -----------------------------------------------------------
>
> (Updated March 9, 2018, 12:17 p.m.)
>
>
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, Jeff Coffler, and Joseph Wu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Many mesos frameworks assume that path separators are forward slashes,
> like dcos-ui and marathon-ui. dcos-cli also assumes this. Previous to
> this patch, if a forward slash was given in the path variable to an HTTP
> API function call to the mesos agent on a Windows system, the Windows
> API would fail at recognizing the path, because the Windows API accepts
> only backslashes as path separators. To remedy this issue, we now
> convert all forward slashes passed as a path to the HTTP API to an agent
> to back slashes on Windows agents by using the path::from_uri function.
>
>
> Diffs
> -----
>
> src/files/files.cpp 5f92d2af722c3201200fa1d6a75dc0b87fdc6078
> src/tests/files_tests.cpp c703cae03345112715aeab83cb0a74abe3e12469
>
>
> Diff: https://reviews.apache.org/r/66002/diff/2/
>
>
> Testing
> -------
>
> Tested on both Windows and Linux, all tests pass, including the two newly enabled tests on Windows.
>
>
> Thanks,
>
> John Kordich
>
>