----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45952/#review137183 -----------------------------------------------------------
Fix it, then Ship it! src/uri/fetchers/docker.cpp (lines 645 - 647) <https://reviews.apache.org/r/45952/#comment202329> I don't get this part. This is not what you're doing here, right? I think a request to authorization server will return an 401 and we will set auth header and send the request again. Putting the comments here is confusing. I would simply add a TODO here saying that here we assume the auth is basic, and we simply include the auth header while sending the request. It'll be ignored if auth is not required. src/uri/fetchers/docker.cpp (line 664) <https://reviews.apache.org/r/45952/#comment202330> This check is redundant, isn't it? I would just remove it. src/uri/fetchers/docker.cpp (line 693) <https://reviews.apache.org/r/45952/#comment202331> I would kill this line. - Jie Yu On June 11, 2016, 12:42 a.m., Gilbert Song wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45952/ > ----------------------------------------------------------- > > (Updated June 11, 2016, 12:42 a.m.) > > > Review request for mesos, Guangya Liu, Artem Harutyunyan, Jie Yu, and Timothy > Chen. > > > Bugs: MESOS-4938 > https://issues.apache.org/jira/browse/MESOS-4938 > > > Repository: mesos > > > Description > ------- > > Implemented support for passing agent default docker config. > > > Diffs > ----- > > src/slave/containerizer/mesos/provisioner/docker/store.cpp > cd5849bb9cdd12f2240885a0eae90569d2a9502e > src/uri/fetchers/docker.hpp c855a2b55a07bb398f7547b44a85b8ba2d2b2ec3 > src/uri/fetchers/docker.cpp ab8f5e05758b7de2573605c81ac80e656bb1db24 > > Diff: https://reviews.apache.org/r/45952/diff/ > > > Testing > ------- > > make check > > Tested with mesos-execute, using a private repo from docker hub. Both cases > are tested: > 1. --docker_registry=private_registry > 2. private_registry/repo > > > Thanks, > > Gilbert Song > >
