Philip Zeyliger 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 1: (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: Docker filesystems, and entrypoint.sh works around it. > I think I understand. Would you file a ticket against me to fix or remove t IMPALA-6737 - bin/bootstrap_development doesn't capture Docker vagaries with Kudu filed. 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: > Now that this can be dealt with by static linking, what are your thoughts? Good instincts on identifying IMPALA-6494 as a static/dynamic linking thing. I should have figured it out, given that I saw a few others of those with boost. 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: # Starts SSH and PostgreSQL; configures container as necessary; > It looks to me like this comment was missed in the last round. I tried for a little bit, and it's not worth it. I ended up producing considerably more lines than the ~4 lines we could be sharing here. Even the hostname change is slightly different in the two contexts, with this context not needing the 127.0.1.1 deletion. http://gerrit.cloudera.org:8080/#/c/9085/1/docker/entrypoint.sh@142 PS1, Line 142: build_shared_libs > That build flag is also implicated in IMPALA-6494, it appears. Yep. I'm ditching shared libs. 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: > I think it may be hesitant in reformatting comments, which may need rewriti I think fixed the rest of these. 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: > This may be an issue of my mediocre Python literacy, but you refer to conta I didn't change the comment after I changed this to pass container objects around. I fixed the comment. I also changed container.id_ to container.id where possible. I don't like using 'id' as a variable name, because it conflicts with built-in id, but I don't see a reason to avoid it as an attribute name. The only awkwardness is around the keyword-args of the constructor, but I think it's clearer without the underscore. 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: # or move to different suite. > nit: can you move these TODOs next to the other ones in this region of comm Done 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: > nit: row Done http://gerrit.cloudera.org:8080/#/c/9085/4/docker/timeline.html.template@41 PS4, Line 41: > Is the unit "percent of CPU used in this container since the previous times Done http://gerrit.cloudera.org:8080/#/c/9085/4/docker/timeline.html.template@51 PS4, Line 51: > What is the string? Done http://gerrit.cloudera.org:8080/#/c/9085/4/docker/timeline.html.template@77 PS4, Line 77: In practice, all of these timestamps are relative, so you'd only get an issue with daylight savings time if the build took us over 2 months (into March of 1970). If Python's time.time() isn't monotonic, we'll get garbage out, but I'm not going to worry about that here. (Python 3.3 has a time.monotonic, but I didn't do the research about py2.7.) I added a comment about why I'm using relative times. (It's really all about tricking the visualization into displaying 01:02:03 for 1h2m3s rather than just 3722 (the number of seconds in my example). Figuring this out made me realize that I need to set yr = 1970 in this string. It didn't really matter, but it makes the function more correct. To answer your question about the function at hand: it ends up being discontinuous and the visualization will look odd. I'd prefer not to go into the rabbit hole of converting the v > new Date(1970, 0, 0, 0, 0, 0, (1520820000 - 1) * 1000) Sun Mar 11 2018 01:59:59 GMT-0800 (PST) > new Date(1970, 0, 0, 0, 0, 0, (1520820000) * 1000) Sun Mar 11 2018 03:00:00 GMT-0700 (PDT) -- 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: 1 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: Wed, 28 Mar 2018 21:18:01 +0000 Gerrit-HasComments: Yes
