impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Zach Amsden (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5764: Allow overriding packaged components
Date Sat, 05 Aug 2017 00:08:49 GMT
Zach Amsden has posted comments on this change.

Change subject: IMPALA-5764: Allow overriding packaged components
......................................................................


Patch Set 1:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/7581/1//COMMIT_MSG
Commit Message:

Line 9: For allowing multiple different distributions to build against the same
> Maybe briefly mention the relevant environment variables so it's more disco
Done


Line 14: 
> can you comment on how other 'packages' can be set up? e.g. what's required
Yeah, these two changes are somewhat intertwined.  I'll put most of the explanation in README.md
rather than the changelog which nobody will read ;)


http://gerrit.cloudera.org:8080/#/c/7581/1/bin/bootstrap_toolchain.py
File bin/bootstrap_toolchain.py:

Line 39: HOST = "https://native-toolchain.s3.amazonaws.com/build"
> It might make sense to also allow configuring this URL, so it can be pointe
I considered it, but it's easy to open a slippery slope to 'Everything is an environment variable!'
and this gets used for both the components and the toolchain with different paths, so I refrained
from letting the change snowball.


PS1, Line 340: llama,
> nit: remove llama (though not llama-minikdc)
Done


PS1, Line 341: minikidc
> typo: minikdc
Done.  Wait - that wasn't even my tyop!  Will fix anyway.


http://gerrit.cloudera.org:8080/#/c/7581/1/bin/impala-config.sh
File bin/impala-config.sh:

PS1, Line 131: cdh
> Keeping it as cdh_components kind-of makes sense to me as long as we're put
My intention was to leave this alone so that the existing workflows for both external and
internal developers continue to work.

The override I had in mind for C6 looks something like:

PACKAGED_COMPONENTS_NAME=cdh6
PACKAGED_COMPONENTS_PATH="/cdh6_components/"
PACKAGED_COMPONENTS="avro,hadoop,hbase,hive,sentry"

Then we can have both C5 & C6 builder jobs run separately and deposit components where
they won't conflict.  Also note we can pull in avro and drop llama-minikdc (or also pull in
hbase-minikdc, if someone is brave enough to set that up)

And of course external thirdparty components are pretty easy to add as well, but with this
arrangement (components override toolchain, as I want to happen for avro), one might argue
for adding the other Apache components directly into the toolchain and letting the build override
them.

But one step at a time, this should suffice for now.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I95c2662e6f62adc924cc5de7a371202126046545
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden <zamsden@cloudera.com>
Gerrit-Reviewer: David Knupp <dknupp@cloudera.com>
Gerrit-Reviewer: Jim Apple <jbapple-impala@apache.org>
Gerrit-Reviewer: Joe McDonnell <joemcdonnell@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Reviewer: Zach Amsden <zamsden@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message