----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52479/#review154340 -----------------------------------------------------------
Fix it, then Ship it! LGTM. docs/features/containers.md (line 107) <https://reviews.apache.org/r/52479/#comment223850> Any particular reason why this is `library/python` instead of just `python` as for the image without binding-helper above? docs/features/containers.md (line 112) <https://reviews.apache.org/r/52479/#comment223849> Please make the clusters.json a link to reference/client-cluster-configuration.md. src/main/python/apache/aurora/client/binding_helpers/docker_helper.py (lines 42 - 43) <https://reviews.apache.org/r/52479/#comment223854> I am new to binding helpers. Do you have a pointer for me to understand why we need both these lines? src/main/python/apache/aurora/client/docker/docker_client.py (lines 41 - 48) <https://reviews.apache.org/r/52479/#comment223858> Domain can be undefined at line 48. src/main/python/apache/aurora/client/docker/docker_client.py (line 57) <https://reviews.apache.org/r/52479/#comment223861> This looks brittle. Would it be possible to make all header comparisions case-insensitive? (I believe it was mentioned before, but I cannot find it right now.) src/main/python/apache/aurora/client/docker/docker_client.py (lines 75 - 82) <https://reviews.apache.org/r/52479/#comment223859> A surprising behaviour of Python is that `headers=dict()` is only run once when the function is defined. The code below that modifies the `headers` dict is thus modyfing the shared object. A common workaround for this is to use `headers=None` in the function definition and assign an empty dict in the function body if `headers is None`. src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh (line 452) <https://reviews.apache.org/r/52479/#comment223860> Have you considered just hardcoding the value in the entire vagrant box? Might make it easier to understand than modifying the global config during the test execution. - Stephan Erb On Oct. 28, 2016, 12:21 a.m., Santhosh Kumar Shanmugham wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52479/ > ----------------------------------------------------------- > > (Updated Oct. 28, 2016, 12:21 a.m.) > > > Review request for Aurora, George Sirois, Joshua Cohen, Stephan Erb, and > Zameer Manji. > > > 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 b050778b5b3b4ab4e40a42cb96fe1851fe1296a5 > build-support/packer/build.sh f5157a60dcabde3e4cd8f6655f1010771f14b702 > docs/features/containers.md 8af38e3af989351925a4130da1ce7a0f5853f0bf > docs/reference/client-cluster-configuration.md > 5a86cdab84049dcbc52021de26acfbc97250295d > examples/jobs/hello_docker_engine.aurora > 3c830e8f99f07732ae985b0aad114c3346888765 > 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/__init__.py PRE-CREATION > src/main/python/apache/aurora/client/docker/docker_client.py 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 > 67702d2c0f2e18ee10dcb798b6d421050bd7d4ca > > 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 > >
