> On Oct. 11, 2016, 2:30 p.m., Joshua Cohen wrote:
> > docs/features/containers.md, line 71
> > <https://reviews.apache.org/r/52479/diff/4/?file=1527964#file1527964line71>
> >
> >     "Starting with the 0.17.0 release, ..."

Done.


> On Oct. 11, 2016, 2:30 p.m., Joshua Cohen wrote:
> > docs/features/containers.md, lines 72-73
> > <https://reviews.apache.org/r/52479/diff/4/?file=1527964#file1527964line72>
> >
> >     "This ensures that the job always uses the same image across restarts, 
> > even if the version identified by the tag has been updated, guaranteeing 
> > that only job updates can mutate configuration."

Done.


> On Oct. 11, 2016, 2:30 p.m., Joshua Cohen wrote:
> > docs/features/containers.md, line 115
> > <https://reviews.apache.org/r/52479/diff/4/?file=1527964#file1527964line115>
> >
> >     "Not, this feature requires a v2 Docker registry."
> >     
> >     (Also lowercase registry throughout.)

Done.


> On Oct. 11, 2016, 2:30 p.m., Joshua Cohen wrote:
> > docs/features/containers.md, line 119
> > <https://reviews.apache.org/r/52479/diff/4/?file=1527964#file1527964line119>
> >
> >     s/which defaults to/and defaults to

Done.


> On Oct. 11, 2016, 2:30 p.m., Joshua Cohen wrote:
> > src/main/python/apache/aurora/client/docker/docker_client.py, line 51
> > <https://reviews.apache.org/r/52479/diff/4/?file=1527973#file1527973line51>
> >
> >     Since we treat all non-200 responses as exceptions, should we just 
> > `raise_for_status` immediately after making the request?

Agree, that is clearer.


> On Oct. 11, 2016, 2:30 p.m., Joshua Cohen wrote:
> > src/main/python/apache/aurora/client/docker/docker_client.py, line 68
> > <https://reviews.apache.org/r/52479/diff/4/?file=1527973#file1527973line68>
> >
> >     Any reason you went with this capitalization? The canonical 
> > capitalization in the RFC is `WWW-Authenticate`, but http headers should 
> > also be case insensitive.
> 
> George Sirois wrote:
>     I think the docker registry actually returns it with that capitalization, 
> but +1 for using a case-insensitive compare if we keep the auth code in this 
> patch.

That is correct. It is an artifact from my curl testing against Docker 
Registry. 'requests' already takes care of this, with a case-insensitive dict.
See 
https://github.com/kennethreitz/requests/blob/5524472cc76ea00d64181505f1fbb7f93f11cc2b/requests/structures.py


> On Oct. 11, 2016, 2:30 p.m., Joshua Cohen wrote:
> > docs/features/containers.md, line 118
> > <https://reviews.apache.org/r/52479/diff/4/?file=1527964#file1527964line118>
> >
> >     s/repositories/registries
> >     s/tp/to
> >     s/docker configuration/Docker configuration

DOne.


> On Oct. 11, 2016, 2:30 p.m., Joshua Cohen wrote:
> > src/main/python/apache/aurora/client/docker/docker_client.py, line 111
> > <https://reviews.apache.org/r/52479/diff/4/?file=1527973#file1527973line111>
> >
> >     maybe use urlparse here?

That's a good suggestion. Done.


> On Oct. 11, 2016, 2:30 p.m., Joshua Cohen wrote:
> > examples/vagrant/provision-dev-cluster.sh, lines 60-70
> > <https://reviews.apache.org/r/52479/diff/4/?file=1527966#file1527966line60>
> >
> >     Does this need to be done during provisioning? Or should it be done as 
> > part of the base box?

The stuff in `build-support/packer/build.sh` seem to be for installing packages 
on the base box. Having said that - this setup here adds some extra time 
(minutes) to the provisioning and hence to the overall end-to-end test. 

Moving the Docker registry setup stuff to `build-support/packer/build.sh`, so 
we can have a cached image in the base box.


- Santhosh Kumar


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/52479/#review152200
-----------------------------------------------------------


On Oct. 12, 2016, 6:52 p.m., Santhosh Kumar Shanmugham wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52479/
> -----------------------------------------------------------
> 
> (Updated Oct. 12, 2016, 6:52 p.m.)
> 
> 
> Review request for Aurora, George Sirois, Joshua Cohen, John Sirois, and 
> Brian Wickman.
> 
> 
> Bugs: AURORA-1014
>     https://issues.apache.org/jira/browse/AURORA-1014
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Resolve docker tags to concrete identifiers for DockerContainerizer
> 
> Docker tags are mutable and can point to different different images
> at different points in time. This makes a job launched with a Docker
> image to be mutable across restarts of the job. This breaks Aurora's
> guarantee of job immutability (except via job updates).
> 
> This change introduces a binding helper, that resolves docker name:tag
> to a concrete registry/name@digest identifier. Identifying
> docker images via a content-addressable digest is available via the
> Docker Registry v2, that is a prerequisite for this feature.
> 
> 
> Diffs
> -----
> 
>   3rdparty/python/requirements.txt 2fb6f4c76c0da9f7958f9b161e701eb894125481 
>   RELEASE-NOTES.md 03349b0b687bade48a2d19712dcb63802f3ef273 
>   build-support/packer/build.sh f5157a60dcabde3e4cd8f6655f1010771f14b702 
>   docs/features/containers.md 8af38e3af989351925a4130da1ce7a0f5853f0bf 
>   examples/jobs/hello_docker_engine.aurora 
> 3c830e8f99f07732ae985b0aad114c3346888765 
>   src/main/python/apache/aurora/client/binding_helpers/BUILD PRE-CREATION 
>   src/main/python/apache/aurora/client/binding_helpers/__init__.py 
> PRE-CREATION 
>   src/main/python/apache/aurora/client/binding_helpers/docker_helper.py 
> PRE-CREATION 
>   src/main/python/apache/aurora/client/cli/client.py 
> fa0c2648c5ff7ea6c9d949cf8cd9b9795d452e98 
>   src/main/python/apache/aurora/client/docker/BUILD PRE-CREATION 
>   src/main/python/apache/aurora/client/docker/__init__.py PRE-CREATION 
>   src/main/python/apache/aurora/client/docker/docker_client.py PRE-CREATION 
>   src/test/python/apache/aurora/client/binding_helpers/BUILD PRE-CREATION 
>   src/test/python/apache/aurora/client/binding_helpers/__init__.py 
> PRE-CREATION 
>   src/test/python/apache/aurora/client/binding_helpers/test_docker_helper.py 
> PRE-CREATION 
>   src/test/python/apache/aurora/client/docker/BUILD PRE-CREATION 
>   src/test/python/apache/aurora/client/docker/__init__.py PRE-CREATION 
>   src/test/python/apache/aurora/client/docker/test_docker_client.py 
> PRE-CREATION 
>   src/test/python/apache/aurora/config/BUILD 
> 52b60409e1de2f69f181a83720ebe1c649b49166 
>   src/test/sh/org/apache/aurora/e2e/http/http_example.aurora 
> c71fb81490863b44827bf090f6e5a6b28da94b93 
>   src/test/sh/org/apache/aurora/e2e/http/http_example_bad_healthcheck.aurora 
> 4fa387d5184968e456bf4c8388496f0514454cb1 
>   src/test/sh/org/apache/aurora/e2e/http/http_example_updated.aurora 
> b279b4f679cc6843e99806bcbf57350ba1c9a6e0 
>   src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 
> c93be9b4b4c76186445ae13eb9424bbf9de9b202 
> 
> Diff: https://reviews.apache.org/r/52479/diff/
> 
> 
> Testing
> -------
> 
> build-support/jenkins/build.sh
> src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Santhosh Kumar Shanmugham
> 
>

Reply via email to