Re: Review Request 36547: Fixed fetcher failing for FTP URIs.
On July 16, 2015, 7:41 p.m., Joerg Schad wrote: src/launcher/fetcher.cpp, line 134 https://reviews.apache.org/r/36547/diff/3/?file=1013405#file1013405line134 I assume those are spaces? Could you please doublecheck? These are spaces. It's probably like this to indicate that the spacing changed. On July 16, 2015, 7:41 p.m., Joerg Schad wrote: src/launcher/fetcher.cpp, line 136 https://reviews.apache.org/r/36547/diff/3/?file=1013405#file1013405line136 can you add a short comment why we assume this here? Added a check to narrow the supported URI protocols. - Jan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/#review91913 --- On July 16, 2015, 6:55 p.m., Jan Schlicht wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/ --- (Updated July 16, 2015, 6:55 p.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3060 https://issues.apache.org/jira/browse/MESOS-3060 Repository: mesos Description --- The response code for successful FTP file transfers is 226, while it is 200 for HTTP. The fetcher has been changed to check for a response code of 226 for FTP URIs. Diffs - src/launcher/fetcher.cpp 8aee4901ec1289f43b5fa6b830c3488815ec24cd Diff: https://reviews.apache.org/r/36547/diff/ Testing --- make check external FTP server test. Thanks, Jan Schlicht
Re: Review Request 36547: Fixed fetcher failing for FTP URIs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/ --- (Updated July 17, 2015, 10:32 a.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3060 https://issues.apache.org/jira/browse/MESOS-3060 Repository: mesos Description --- The response code for successful FTP file transfers is 226, while it is 200 for HTTP. The fetcher has been changed to check for a response code of 226 for FTP URIs. Diffs (updated) - src/launcher/fetcher.cpp 8aee4901ec1289f43b5fa6b830c3488815ec24cd Diff: https://reviews.apache.org/r/36547/diff/ Testing --- make check external FTP server test. Thanks, Jan Schlicht
Re: Review Request 36547: Fixed fetcher failing for FTP URIs.
On July 17, 2015, 4:59 a.m., Adam B wrote: src/launcher/fetcher.cpp, lines 127-128 https://reviews.apache.org/r/36547/diff/3/?file=1013405#file1013405line127 Why not just check for any 2xx code then? Aren't they all successful in one way or another? HTTP can return 206, which means that only a part of the content has been (successfully) transferred. Therefore we should really check for the designated return codes that indicate a successful transfer of a whole resource. - Jan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/#review92015 --- On July 16, 2015, 6:55 p.m., Jan Schlicht wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/ --- (Updated July 16, 2015, 6:55 p.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3060 https://issues.apache.org/jira/browse/MESOS-3060 Repository: mesos Description --- The response code for successful FTP file transfers is 226, while it is 200 for HTTP. The fetcher has been changed to check for a response code of 226 for FTP URIs. Diffs - src/launcher/fetcher.cpp 8aee4901ec1289f43b5fa6b830c3488815ec24cd Diff: https://reviews.apache.org/r/36547/diff/ Testing --- make check external FTP server test. Thanks, Jan Schlicht
Re: Review Request 36547: Fixed fetcher failing for FTP URIs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/ --- (Updated July 17, 2015, 10:56 a.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Changes --- Remove redundant CHECK. Bugs: MESOS-3060 https://issues.apache.org/jira/browse/MESOS-3060 Repository: mesos Description --- The response code for successful FTP file transfers is 226, while it is 200 for HTTP. The fetcher has been changed to check for a response code of 226 for FTP URIs. Diffs (updated) - src/launcher/fetcher.cpp 8aee4901ec1289f43b5fa6b830c3488815ec24cd Diff: https://reviews.apache.org/r/36547/diff/ Testing --- make check external FTP server test. Thanks, Jan Schlicht
Re: Review Request 36547: Fixed fetcher failing for FTP URIs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/#review92046 --- src/launcher/fetcher.cpp (line 126) https://reviews.apache.org/r/36547/#comment145935 There is an extra blank that does not belong. - Bernd Mathiske On July 17, 2015, 1:56 a.m., Jan Schlicht wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/ --- (Updated July 17, 2015, 1:56 a.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3060 https://issues.apache.org/jira/browse/MESOS-3060 Repository: mesos Description --- The response code for successful FTP file transfers is 226, while it is 200 for HTTP. The fetcher has been changed to check for a response code of 226 for FTP URIs. Diffs - src/launcher/fetcher.cpp 8aee4901ec1289f43b5fa6b830c3488815ec24cd Diff: https://reviews.apache.org/r/36547/diff/ Testing --- make check external FTP server test. Thanks, Jan Schlicht
Re: Review Request 36547: Fixed fetcher failing for FTP URIs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/ --- (Updated July 17, 2015, 10:40 a.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3060 https://issues.apache.org/jira/browse/MESOS-3060 Repository: mesos Description --- The response code for successful FTP file transfers is 226, while it is 200 for HTTP. The fetcher has been changed to check for a response code of 226 for FTP URIs. Diffs (updated) - src/launcher/fetcher.cpp 8aee4901ec1289f43b5fa6b830c3488815ec24cd Diff: https://reviews.apache.org/r/36547/diff/ Testing --- make check external FTP server test. Thanks, Jan Schlicht
Re: Review Request 36547: Fixed fetcher failing for FTP URIs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/#review92042 --- Ship it! Ship It! - Joerg Schad On July 17, 2015, 8:40 a.m., Jan Schlicht wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/ --- (Updated July 17, 2015, 8:40 a.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3060 https://issues.apache.org/jira/browse/MESOS-3060 Repository: mesos Description --- The response code for successful FTP file transfers is 226, while it is 200 for HTTP. The fetcher has been changed to check for a response code of 226 for FTP URIs. Diffs - src/launcher/fetcher.cpp 8aee4901ec1289f43b5fa6b830c3488815ec24cd Diff: https://reviews.apache.org/r/36547/diff/ Testing --- make check external FTP server test. Thanks, Jan Schlicht
Re: Review Request 36547: Fixed fetcher failing for FTP URIs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/#review92045 --- Ship it! Ship It! - Joerg Schad On July 17, 2015, 8:56 a.m., Jan Schlicht wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/ --- (Updated July 17, 2015, 8:56 a.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3060 https://issues.apache.org/jira/browse/MESOS-3060 Repository: mesos Description --- The response code for successful FTP file transfers is 226, while it is 200 for HTTP. The fetcher has been changed to check for a response code of 226 for FTP URIs. Diffs - src/launcher/fetcher.cpp 8aee4901ec1289f43b5fa6b830c3488815ec24cd Diff: https://reviews.apache.org/r/36547/diff/ Testing --- make check external FTP server test. Thanks, Jan Schlicht
Re: Review Request 36547: Fixed fetcher failing for FTP URIs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/ --- (Updated July 17, 2015, 11:05 a.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3060 https://issues.apache.org/jira/browse/MESOS-3060 Repository: mesos Description --- The response code for successful FTP file transfers is 226, while it is 200 for HTTP. The fetcher has been changed to check for a response code of 226 for FTP URIs. Diffs (updated) - src/launcher/fetcher.cpp 8aee4901ec1289f43b5fa6b830c3488815ec24cd Diff: https://reviews.apache.org/r/36547/diff/ Testing --- make check external FTP server test. Thanks, Jan Schlicht
Re: Review Request 36547: Fixed fetcher failing for FTP URIs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/#review92048 --- Ship it! Ship It! - Joerg Schad On July 17, 2015, 9:05 a.m., Jan Schlicht wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/ --- (Updated July 17, 2015, 9:05 a.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3060 https://issues.apache.org/jira/browse/MESOS-3060 Repository: mesos Description --- The response code for successful FTP file transfers is 226, while it is 200 for HTTP. The fetcher has been changed to check for a response code of 226 for FTP URIs. Diffs - src/launcher/fetcher.cpp 8aee4901ec1289f43b5fa6b830c3488815ec24cd Diff: https://reviews.apache.org/r/36547/diff/ Testing --- make check external FTP server test. Thanks, Jan Schlicht
Re: Review Request 36547: Fixed fetcher failing for FTP URIs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/#review92051 --- Patch looks great! Reviews applied: [36547] All tests passed. - Mesos ReviewBot On July 17, 2015, 9:05 a.m., Jan Schlicht wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/ --- (Updated July 17, 2015, 9:05 a.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3060 https://issues.apache.org/jira/browse/MESOS-3060 Repository: mesos Description --- The response code for successful FTP file transfers is 226, while it is 200 for HTTP. The fetcher has been changed to check for a response code of 226 for FTP URIs. Diffs - src/launcher/fetcher.cpp 8aee4901ec1289f43b5fa6b830c3488815ec24cd Diff: https://reviews.apache.org/r/36547/diff/ Testing --- make check external FTP server test. Thanks, Jan Schlicht
Re: Review Request 36547: Fixed fetcher failing for FTP URIs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/#review92052 --- Ship it! Ship It! - Bernd Mathiske On July 17, 2015, 2:05 a.m., Jan Schlicht wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/ --- (Updated July 17, 2015, 2:05 a.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3060 https://issues.apache.org/jira/browse/MESOS-3060 Repository: mesos Description --- The response code for successful FTP file transfers is 226, while it is 200 for HTTP. The fetcher has been changed to check for a response code of 226 for FTP URIs. Diffs - src/launcher/fetcher.cpp 8aee4901ec1289f43b5fa6b830c3488815ec24cd Diff: https://reviews.apache.org/r/36547/diff/ Testing --- make check external FTP server test. Thanks, Jan Schlicht
Re: Review Request 36547: Fixed fetcher failing for FTP URIs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/#review91909 --- Patch looks great! Reviews applied: [36547] All tests passed. - Mesos ReviewBot On July 16, 2015, 4:55 p.m., Jan Schlicht wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/ --- (Updated July 16, 2015, 4:55 p.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3060 https://issues.apache.org/jira/browse/MESOS-3060 Repository: mesos Description --- The response code for successful FTP file transfers is 226, while it is 200 for HTTP. The fetcher has been changed to check for a response code of 226 for FTP URIs. Diffs - src/launcher/fetcher.cpp 8aee4901ec1289f43b5fa6b830c3488815ec24cd Diff: https://reviews.apache.org/r/36547/diff/ Testing --- make check external FTP server test. Thanks, Jan Schlicht
Re: Review Request 36547: Fixed fetcher failing for FTP URIs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/#review91913 --- Ship it! src/launcher/fetcher.cpp (line 134) https://reviews.apache.org/r/36547/#comment145628 I assume those are spaces? Could you please doublecheck? src/launcher/fetcher.cpp (line 136) https://reviews.apache.org/r/36547/#comment145626 can you add a short comment why we assume this here? - Joerg Schad On July 16, 2015, 4:55 p.m., Jan Schlicht wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/ --- (Updated July 16, 2015, 4:55 p.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3060 https://issues.apache.org/jira/browse/MESOS-3060 Repository: mesos Description --- The response code for successful FTP file transfers is 226, while it is 200 for HTTP. The fetcher has been changed to check for a response code of 226 for FTP URIs. Diffs - src/launcher/fetcher.cpp 8aee4901ec1289f43b5fa6b830c3488815ec24cd Diff: https://reviews.apache.org/r/36547/diff/ Testing --- make check external FTP server test. Thanks, Jan Schlicht
Re: Review Request 36547: Fixed fetcher failing for FTP URIs.
On July 16, 2015, 2:37 p.m., Joerg Schad wrote: src/launcher/fetcher.cpp, line 128 https://reviews.apache.org/r/36547/diff/1/?file=1013402#file1013402line128 indentation +2 spaces (https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Conditionals) see line 61 int he same file for an example :-) - Joerg --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/#review91885 --- On July 16, 2015, 2:25 p.m., Jan Schlicht wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/ --- (Updated July 16, 2015, 2:25 p.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3060 https://issues.apache.org/jira/browse/MESOS-3060 Repository: mesos Description --- The response code for successful FTP file transfers is 226, while it is 200 for HTTP. The fetcher has been changed to check for a response code of 226 for FTP URIs. Diffs - src/launcher/fetcher.cpp 8aee4901ec1289f43b5fa6b830c3488815ec24cd Diff: https://reviews.apache.org/r/36547/diff/ Testing --- make check external FTP server test. Thanks, Jan Schlicht
Re: Review Request 36547: Fixed fetcher failing for FTP URIs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/ --- (Updated July 16, 2015, 2:48 p.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3060 https://issues.apache.org/jira/browse/MESOS-3060 Repository: mesos Description --- The response code for successful FTP file transfers is 226, while it is 200 for HTTP. The fetcher has been changed to check for a response code of 226 for FTP URIs. Diffs (updated) - src/launcher/fetcher.cpp 8aee4901ec1289f43b5fa6b830c3488815ec24cd Diff: https://reviews.apache.org/r/36547/diff/ Testing --- make check external FTP server test. Thanks, Jan Schlicht
Re: Review Request 36547: Fixed fetcher failing for FTP URIs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/#review91885 --- src/launcher/fetcher.cpp (line 123) https://reviews.apache.org/r/36547/#comment145590 s\response\status (to be consistent with below) src/launcher/fetcher.cpp (line 125) https://reviews.apache.org/r/36547/#comment145589 s\successCode\successStatusCode src/launcher/fetcher.cpp (line 128) https://reviews.apache.org/r/36547/#comment145588 indentation +2 spaces (https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Conditionals) - Joerg Schad On July 16, 2015, 2:25 p.m., Jan Schlicht wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/ --- (Updated July 16, 2015, 2:25 p.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3060 https://issues.apache.org/jira/browse/MESOS-3060 Repository: mesos Description --- The response code for successful FTP file transfers is 226, while it is 200 for HTTP. The fetcher has been changed to check for a response code of 226 for FTP URIs. Diffs - src/launcher/fetcher.cpp 8aee4901ec1289f43b5fa6b830c3488815ec24cd Diff: https://reviews.apache.org/r/36547/diff/ Testing --- make check external FTP server test. Thanks, Jan Schlicht
Re: Review Request 36547: Fixed fetcher failing for FTP URIs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/#review91891 --- src/launcher/fetcher.cpp (line 135) https://reviews.apache.org/r/36547/#comment145601 Suggestion: put the condition check for the protocol AFTER checking if we have an error. Then we do not need the comment above nor the extra variable AND we can state in the error message which specific protocol is meant instead of HTTP/FTP. - Bernd Mathiske On July 16, 2015, 7:48 a.m., Jan Schlicht wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/ --- (Updated July 16, 2015, 7:48 a.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3060 https://issues.apache.org/jira/browse/MESOS-3060 Repository: mesos Description --- The response code for successful FTP file transfers is 226, while it is 200 for HTTP. The fetcher has been changed to check for a response code of 226 for FTP URIs. Diffs - src/launcher/fetcher.cpp 8aee4901ec1289f43b5fa6b830c3488815ec24cd Diff: https://reviews.apache.org/r/36547/diff/ Testing --- make check external FTP server test. Thanks, Jan Schlicht
Re: Review Request 36547: Fixed fetcher failing for FTP URIs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/#review91892 --- Patch looks great! Reviews applied: [36547] All tests passed. - Mesos ReviewBot On July 16, 2015, 2:48 p.m., Jan Schlicht wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/ --- (Updated July 16, 2015, 2:48 p.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3060 https://issues.apache.org/jira/browse/MESOS-3060 Repository: mesos Description --- The response code for successful FTP file transfers is 226, while it is 200 for HTTP. The fetcher has been changed to check for a response code of 226 for FTP URIs. Diffs - src/launcher/fetcher.cpp 8aee4901ec1289f43b5fa6b830c3488815ec24cd Diff: https://reviews.apache.org/r/36547/diff/ Testing --- make check external FTP server test. Thanks, Jan Schlicht
Re: Review Request 36547: Fixed fetcher failing for FTP URIs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/#review91904 --- Ship it! Ship It! - Bernd Mathiske On July 16, 2015, 9:55 a.m., Jan Schlicht wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/ --- (Updated July 16, 2015, 9:55 a.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3060 https://issues.apache.org/jira/browse/MESOS-3060 Repository: mesos Description --- The response code for successful FTP file transfers is 226, while it is 200 for HTTP. The fetcher has been changed to check for a response code of 226 for FTP URIs. Diffs - src/launcher/fetcher.cpp 8aee4901ec1289f43b5fa6b830c3488815ec24cd Diff: https://reviews.apache.org/r/36547/diff/ Testing --- make check external FTP server test. Thanks, Jan Schlicht
Re: Review Request 36547: Fixed fetcher failing for FTP URIs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/#review92015 --- src/launcher/fetcher.cpp (lines 127 - 128) https://reviews.apache.org/r/36547/#comment145870 Why not just check for any 2xx code then? Aren't they all successful in one way or another? - Adam B On July 16, 2015, 9:55 a.m., Jan Schlicht wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36547/ --- (Updated July 16, 2015, 9:55 a.m.) Review request for mesos, Bernd Mathiske and Joerg Schad. Bugs: MESOS-3060 https://issues.apache.org/jira/browse/MESOS-3060 Repository: mesos Description --- The response code for successful FTP file transfers is 226, while it is 200 for HTTP. The fetcher has been changed to check for a response code of 226 for FTP URIs. Diffs - src/launcher/fetcher.cpp 8aee4901ec1289f43b5fa6b830c3488815ec24cd Diff: https://reviews.apache.org/r/36547/diff/ Testing --- make check external FTP server test. Thanks, Jan Schlicht
