> On Oct. 31, 2016, 4:35 p.m., Stephan Erb wrote: > > src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh, line 453 > > <https://reviews.apache.org/r/52479/diff/8/?file=1547811#file1547811line453> > > > > 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.
Both the `end-to-end` tests and the `examples` use the same aurora command and hence will depend on the same `clusters.json` file. So I chose to modify the config on the fly so that the client will talk to the local Docker registry while executing the tests and talk to the public Docker Hub while executing the examples. > On Oct. 31, 2016, 4:35 p.m., Stephan Erb wrote: > > src/main/python/apache/aurora/client/docker/docker_client.py, line 57 > > <https://reviews.apache.org/r/52479/diff/8/?file=1547801#file1547801line57> > > > > 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.) Yes, it was brought up before and answered. Fortunately, 'requests' already takes care of this, with a case-insensitive dict. See https://github.com/kennethreitz/requests/blob/5524472cc76ea00d64181505f1fbb7f93f11cc2b/requests/structures.py > On Oct. 31, 2016, 4:35 p.m., Stephan Erb wrote: > > docs/features/containers.md, line 107 > > <https://reviews.apache.org/r/52479/diff/8/?file=1547794#file1547794line107> > > > > Any particular reason why this is `library/python` instead of just > > `python` as for the image without binding-helper above? Docker Registry has the concept of namespaces, where `library` is a restricted namespace that will hold all the official images. Docker Engine provides a convenience (or probably backward compatible?) feature which allows for aliasing these official images without the namespace. In essense, `ubuntu` is an alias to `library/ubuntu`. Since the Docker binder can potentially communicate with privately hosted registries, I chose not to have any logic that is specific to the Docker Hub. > On Oct. 31, 2016, 4:35 p.m., Stephan Erb wrote: > > docs/features/containers.md, line 112 > > <https://reviews.apache.org/r/52479/diff/8/?file=1547794#file1547794line112> > > > > Please make the clusters.json a link to > > reference/client-cluster-configuration.md. Done. > On Oct. 31, 2016, 4:35 p.m., Stephan Erb wrote: > > src/main/python/apache/aurora/client/docker/docker_client.py, lines 41-48 > > <https://reviews.apache.org/r/52479/diff/8/?file=1547801#file1547801line41> > > > > Domain can be undefined at line 48. Fixed. > On Oct. 31, 2016, 4:35 p.m., Stephan Erb wrote: > > src/main/python/apache/aurora/client/docker/docker_client.py, lines 75-82 > > <https://reviews.apache.org/r/52479/diff/8/?file=1547801#file1547801line75> > > > > 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`. Did not know that. Fixed. > On Oct. 31, 2016, 4:35 p.m., Stephan Erb wrote: > > src/main/python/apache/aurora/client/binding_helpers/docker_helper.py, > > lines 42-43 > > <https://reviews.apache.org/r/52479/diff/8/?file=1547798#file1547798line42> > > > > I am new to binding helpers. Do you have a pointer for me to understand > > why we need both these lines? Bindings can be resolved from scratch or from a dictionary (which can be communicated over the network). Although there is no use for the latter, I am following the design here. - Santhosh Kumar ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52479/#review154340 ----------------------------------------------------------- On Oct. 27, 2016, 3:21 p.m., Santhosh Kumar Shanmugham wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/52479/ > ----------------------------------------------------------- > > (Updated Oct. 27, 2016, 3:21 p.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 > >
