Re: Review Request 68297: Windows: Fixed a bug when `executorId` contains a `:`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68297/ --- (Updated Aug. 17, 2018, 6:18 p.m.) Review request for mesos, Benjamin Mahler, Gastón Kleiman, and Joseph Wu. Bugs: MESOS-9109 https://issues.apache.org/jira/browse/MESOS-9109 Repository: mesos Description (updated) --- Per MESOS-9109, some frameworks, such as Chronos, use a `:` in their `executorId`, but this is a reserved character on Windows, and when its used as part of the path on disk results in an OS error. Unfortunately, we need to remain backward-compatible with existing frameworks, so now we escape `:` as `%3A` when writing to disk, and unescape it when parsing. Diffs (updated) - src/slave/paths.cpp ed0b1276908f4990ce7a24c96aea20e8c79d3126 src/tests/paths_tests.cpp 4808402f789ebc783b308c03b8e412ba22bf9f5d Diff: https://reviews.apache.org/r/68297/diff/3/ Changes: https://reviews.apache.org/r/68297/diff/2-3/ Testing --- ``` [--] Global test environment tear-down [==] 1008 tests from 98 test cases ran. (446260 ms total) [ PASSED ] 1008 tests. ``` Note that this is without changing the used-everywhere default executor ID of `default` to `default:id`, which I also tested, but required changes of asserts (and caused long path issues only solvable by changing the machine's registry, so I'd rather not include it). Thanks, Andrew Schwartzmeyer
Re: Review Request 68297: Windows: Fixed a bug when `executorId` contains a `:`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68297/ --- (Updated Aug. 15, 2018, 8:12 p.m.) Review request for mesos, Benjamin Mahler, Gastón Kleiman, and Joseph Wu. Changes --- Switched to `%3A`. TODO: Maybe use `process::http::encode` here instead. Bugs: MESOS-9109 https://issues.apache.org/jira/browse/MESOS-9109 Repository: mesos Description --- Per MESOS-9109, some frameworks, such as Chronos, use a `:` in their `executorId`, but this is a reserved character on Windows, and when its use as part of the path on disk results in an OS error. Unfortunately, we need to remain backward-compatible with existing frameworks, so now we escape `:` as `_COLON_` when writing to disk, and unescape it when parsing. Note that we explicitly do not escape to `%3A` (the ASCII equivalent), because otherwise it is impossible to query for the file over HTTP, as libprocess helpfully (but not in this case) pre-emptively decodes it to `:` and so queries for the incorrect path. Diffs (updated) - src/slave/paths.cpp ed0b1276908f4990ce7a24c96aea20e8c79d3126 src/tests/paths_tests.cpp 4808402f789ebc783b308c03b8e412ba22bf9f5d Diff: https://reviews.apache.org/r/68297/diff/2/ Changes: https://reviews.apache.org/r/68297/diff/1-2/ Testing --- ``` [--] Global test environment tear-down [==] 1008 tests from 98 test cases ran. (446260 ms total) [ PASSED ] 1008 tests. ``` Note that this is without changing the used-everywhere default executor ID of `default` to `default:id`, which I also tested, but required changes of asserts (and caused long path issues only solvable by changing the machine's registry, so I'd rather not include it). Thanks, Andrew Schwartzmeyer
Re: Review Request 68297: Windows: Fixed a bug when `executorId` contains a `:`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68297/#review207084 --- PASS: Mesos patch 68297 was successfully built and tested. Reviews applied: `['68297']` All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2128/mesos-review-68297 - Mesos Reviewbot Windows On Aug. 10, 2018, 5:26 p.m., Andrew Schwartzmeyer wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68297/ > --- > > (Updated Aug. 10, 2018, 5:26 p.m.) > > > Review request for mesos, Benjamin Mahler, Gastón Kleiman, and Joseph Wu. > > > Bugs: MESOS-9109 > https://issues.apache.org/jira/browse/MESOS-9109 > > > Repository: mesos > > > Description > --- > > Per MESOS-9109, some frameworks, such as Chronos, use a `:` in their > `executorId`, but this is a reserved character on Windows, and when > its use as part of the path on disk results in an OS error. > Unfortunately, we need to remain backward-compatible with existing > frameworks, so now we escape `:` as `_COLON_` when writing to disk, > and unescape it when parsing. > > Note that we explicitly do not escape to `%3A` (the ASCII equivalent), > because otherwise it is impossible to query for the file over HTTP, as > libprocess helpfully (but not in this case) pre-emptively decodes it > to `:` and so queries for the incorrect path. > > > Diffs > - > > src/slave/paths.cpp ed0b1276908f4990ce7a24c96aea20e8c79d3126 > src/tests/paths_tests.cpp 4808402f789ebc783b308c03b8e412ba22bf9f5d > > > Diff: https://reviews.apache.org/r/68297/diff/1/ > > > Testing > --- > > ``` > [--] Global test environment tear-down > [==] 1008 tests from 98 test cases ran. (446260 ms total) > [ PASSED ] 1008 tests. > ``` > > Note that this is without changing the used-everywhere default executor ID of > `default` to `default:id`, which I also tested, but required changes of > asserts (and caused long path issues only solvable by changing the machine's > registry, so I'd rather not include it). > > > Thanks, > > Andrew Schwartzmeyer > >
Review Request 68297: Windows: Fixed a bug when `executorId` contains a `:`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68297/ --- Review request for mesos, Benjamin Mahler, Gastón Kleiman, and Joseph Wu. Bugs: MESOS-9109 https://issues.apache.org/jira/browse/MESOS-9109 Repository: mesos Description --- Per MESOS-9109, some frameworks, such as Chronos, use a `:` in their `executorId`, but this is a reserved character on Windows, and when its use as part of the path on disk results in an OS error. Unfortunately, we need to remain backward-compatible with existing frameworks, so now we escape `:` as `_COLON_` when writing to disk, and unescape it when parsing. Note that we explicitly do not escape to `%3A` (the ASCII equivalent), because otherwise it is impossible to query for the file over HTTP, as libprocess helpfully (but not in this case) pre-emptively decodes it to `:` and so queries for the incorrect path. Diffs - src/slave/paths.cpp ed0b1276908f4990ce7a24c96aea20e8c79d3126 src/tests/paths_tests.cpp 4808402f789ebc783b308c03b8e412ba22bf9f5d Diff: https://reviews.apache.org/r/68297/diff/1/ Testing --- ``` [--] Global test environment tear-down [==] 1008 tests from 98 test cases ran. (446260 ms total) [ PASSED ] 1008 tests. ``` Note that this is without changing the used-everywhere default executor ID of `default` to `default:id`, which I also tested, but required changes of asserts (and caused long path issues only solvable by changing the machine's registry, so I'd rather not include it). Thanks, Andrew Schwartzmeyer
