impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Lars Volker (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-4047: DO NOT SUBMIT Highlight all occurrences of CDH/cdh
Date Thu, 29 Sep 2016 11:22:39 GMT
Lars Volker has posted comments on this change.

Change subject: IMPALA-4047: DO NOT SUBMIT Highlight all occurrences of CDH/cdh

Patch Set 5:


Thanks for all the help so far. I created a bunch of upstream Jiras and replaced references
to internal ones.

Please see my inline comments.
File bin/

PS4, Line 24: 
Can we rename this to DOWNLOAD_HADOOP_COMPONENTS instead? Or are they CDH specific versions
and that would make it worse?
File bin/

Line 46
This was unused, removed
File bin/

PS1, Line 79: 
Do we have plans to actually get rid of the CDH component dependencies? This one seems to
affect a lot more occurrences below. If anyone can shed some light on it I'll go ahead and
create a Jira for doing it.
File bin/

Line 144
> There are so many different things we are doing with different "CDH"s, it m
Created IMPALA-4208 to track this.
File bin/

PS4, Line 77: 
see comments in
File bin/

Line 24
This has been fixed separately in IMPALA-4116
File bin/

PS4, Line 220: 
File common/thrift/

PS1, Line 49: parser.add_option("--output_mdl_version", dest="output_mdl_version",
            :                   metavar="IMPALA_VERSION", default="2.8.0-SNAPSHOT",
            :                   help="The Impala version that is written in the output mdl.")
> Thanks. Let me know what they say and I'll create a Jira to track removal o
Did anything come out of this? For now I changed the version to the current version in,
so the 'cdh' is gone. :)
File common/thrift/

Line 50
I think we should have better scripts for version management, i.e. one script that produces
proper version files for all languages to include. For now I will bump this to the current
version in
File ext-data-source/api/pom.xml:

File ext-data-source/sample/pom.xml:

File ext-data-source/test/pom.xml:

File fe/pom.xml:

File infra/deploy/

PS4, Line 23: 
I doubt this is accurate, otherwise it could probably be removed, since this quite an old

@mj, can you have a look? Should we move this script to some internal repo or is it useful
without CDH, too?

Let me know what to do and I'll open a Jira to track.
File testdata/cluster/.gitignore:

PS3, Line 1: kud
> I'm not sure about this one. I don't want to have to add one of these for e
Good point, removed it.
File testdata/cluster/admin:

> Yes, I think I was wrong. There was a similar discussion in bin/start-impal
File testdata/cluster/admin:


PS4, Line 28: 
File testdata/datasets/functional/schema_constraints.csv:

PS4, Line 126: 
Wouldn't this hold true for all versions of CDH (and all environments, really)? Then again
I don't understand why the comment is there in the first place. Can decimal not be tested
File tests/comparison/

File tests/comparison/leopard/

Line 34: DEFAULT_BRANCH_NAME = 'origin/cdh5-trunk'
> Whether IMPALA-4085 is fixed, we should remove these.
I don't know, I will ask on the mailing list.

By "remove these", do you mean the variables? Or the whole file?
File tests/metadata/

PS4, Line 45: 
I reworded this.
File tests/query_test/

Line 38
This seems to be Hive < 0.11, so I will reword this.
File tests/test-hive-udfs/pom.xml:


To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb37e2ef0cd9fa0e581d359c5dd3db7812b7b2c8
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: Jim Apple <>
Gerrit-Reviewer: Lars Volker <>
Gerrit-Reviewer: Matthew Jacobs <>
Gerrit-Reviewer: Michael Brown <>
Gerrit-HasComments: Yes

View raw message