You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Laszlo Gaal (Code Review)" <ge...@cloudera.org> on 2019/02/05 15:53:20 UTC

[native-toolchain-CR] Initial support for building the toolchain in docker

Laszlo Gaal has posted comments on this change. ( http://gerrit.cloudera.org:8080/12285 )

Change subject: Initial support for building the toolchain in docker
......................................................................


Patch Set 4:

(3 comments)

I am still concerned about version pinning for the Docker images. The base images look all right, but the subsequent install steps will still use the "latest" repos for the CentOS images: the repo descriptors in /etc/yum.repos.d use only the OS major version in their default state, so all the packages will be installed from a much later minor version than the base images themselves. I wonder if we are setting ourselves up for later problems this way.

http://gerrit.cloudera.org:8080/#/c/12285/2/Makefile
File Makefile:

http://gerrit.cloudera.org:8080/#/c/12285/2/Makefile@11
PS2, Line 11:  WITHOUT
nit: you probably want to add this in the follow-up patch (the one adding Ubuntu18 support).


http://gerrit.cloudera.org:8080/#/c/12285/4/Makefile
File Makefile:

http://gerrit.cloudera.org:8080/#/c/12285/4/Makefile@25
PS4, Line 25: ubuntu18
Maybe move this to the Ubuntu18 support commit?


http://gerrit.cloudera.org:8080/#/c/12285/4/in-docker.py
File in-docker.py:

http://gerrit.cloudera.org:8080/#/c/12285/4/in-docker.py@64
PS4, Line 64: impal
SImilarly, maybe this should also be moved to the other commit that enables Ubuntu18.



-- 
To view, visit http://gerrit.cloudera.org:8080/12285
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: native-toolchain
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If42c9bc06a3d303642eb37dea784b61e2a1f5cc6
Gerrit-Change-Number: 12285
Gerrit-PatchSet: 4
Gerrit-Owner: hector.acosta@cloudera.com <he...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Laszlo Gaal <la...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: hector.acosta@cloudera.com <he...@cloudera.com>
Gerrit-Comment-Date: Tue, 05 Feb 2019 15:53:20 +0000
Gerrit-HasComments: Yes