Jim Apple has posted comments on this change. ( http://gerrit.cloudera.org:8080/9085 )
Change subject: IMPALA-6070: Expose using Docker to run tests faster. ...................................................................... Patch Set 4: (11 comments) http://gerrit.cloudera.org:8080/#/c/9085/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9085/1//COMMIT_MSG@28 PS1, Line 28: Besides the obvious win of getting test results more quickly, this > Here are a few pointers. I think I understand. Would you file a ticket against me to fix or remove the docker instructions in bin/bootstrap_development.sh? http://gerrit.cloudera.org:8080/#/c/9085/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9085/3//COMMIT_MSG@48 PS3, Line 48: IMPALA-6494 > I agree that this is a blocker for saying turning off GVOs as they are toda Now that this can be dealt with by static linking, what are your thoughts? http://gerrit.cloudera.org:8080/#/c/9085/1/docker/entrypoint.sh File docker/entrypoint.sh: http://gerrit.cloudera.org:8080/#/c/9085/1/docker/entrypoint.sh@79 PS1, Line 79: fi > I think there is more overlap. The hosts munging is in both places, for ins It looks to me like this comment was missed in the last round. http://gerrit.cloudera.org:8080/#/c/9085/1/docker/entrypoint.sh@142 PS1, Line 142: ably because ther > I'd suggest changing GVD back for some time while you and I work through th That build flag is also implicated in IMPALA-6494, it appears. http://gerrit.cloudera.org:8080/#/c/9085/3/docker/monitor.py File docker/monitor.py: http://gerrit.cloudera.org:8080/#/c/9085/3/docker/monitor.py@221 PS3, Line 221: line.strip() > Done. I'm a little bit confused about why the autopep8 thing you mentioned I think it may be hesitant in reformatting comments, which may need rewriting. "Multi-line docstrings consist of a summary line just like a one-line docstring, followed by a blank line, followed by a more elaborate description." That summary line is normally just a single line, so this docstring and one above are not in the convention. http://gerrit.cloudera.org:8080/#/c/9085/4/docker/monitor.py File docker/monitor.py: http://gerrit.cloudera.org:8080/#/c/9085/4/docker/monitor.py@149 PS4, Line 149: container (again, a path) This may be an issue of my mediocre Python literacy, but you refer to container.id_, but I don't see id_ on https://docs.python.org/2/library/os.path.html http://gerrit.cloudera.org:8080/#/c/9085/4/docker/test-with-docker.py File docker/test-with-docker.py: http://gerrit.cloudera.org:8080/#/c/9085/4/docker/test-with-docker.py@49 PS4, Line 49: # Suggested speed improvement TODOs: nit: can you move these TODOs next to the other ones in this region of comments? http://gerrit.cloudera.org:8080/#/c/9085/4/docker/timeline.html.template File docker/timeline.html.template: http://gerrit.cloudera.org:8080/#/c/9085/4/docker/timeline.html.template@23 PS4, Line 23: ow nit: row http://gerrit.cloudera.org:8080/#/c/9085/4/docker/timeline.html.template@41 PS4, Line 41: user CPU Is the unit "percent of CPU used in this container since the previous timestamp"? http://gerrit.cloudera.org:8080/#/c/9085/4/docker/timeline.html.template@51 PS4, Line 51: the string What is the string? http://gerrit.cloudera.org:8080/#/c/9085/4/docker/timeline.html.template@77 PS4, Line 77: browser local time super-nit: what happens for builds when the clock in the browser local time was non-monotonic or jumped ahead, as in DST? -- To view, visit http://gerrit.cloudera.org:8080/9085 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I82052ef31979564968effef13a3c6af0d5c62767 Gerrit-Change-Number: 9085 Gerrit-PatchSet: 4 Gerrit-Owner: Philip Zeyliger <[email protected]> Gerrit-Reviewer: Jim Apple <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Philip Zeyliger <[email protected]> Gerrit-Comment-Date: Sun, 25 Mar 2018 00:54:39 +0000 Gerrit-HasComments: Yes
