Lars Volker has posted comments on this change. Change subject: IMPALA-4047: DO NOT SUBMIT Highlight all occurrences of CDH/cdh ......................................................................
Patch Set 5: (24 comments) 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. http://gerrit.cloudera.org:8080/#/c/4187/4/bin/bootstrap_toolchain.py File bin/bootstrap_toolchain.py: PS4, Line 24: Can we rename this to DOWNLOAD_HADOOP_COMPONENTS instead? Or are they CDH specific versions and that would make it worse? http://gerrit.cloudera.org:8080/#/c/4187/4/bin/build_thirdparty.sh File bin/build_thirdparty.sh: Line 46 This was unused, removed http://gerrit.cloudera.org:8080/#/c/4187/1/bin/impala-config.sh File bin/impala-config.sh: 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. http://gerrit.cloudera.org:8080/#/c/4187/3/bin/impala-config.sh File bin/impala-config.sh: Line 144 > There are so many different things we are doing with different "CDH"s, it m Created IMPALA-4208 to track this. http://gerrit.cloudera.org:8080/#/c/4187/4/bin/impala-config.sh File bin/impala-config.sh: PS4, Line 77: see comments in bootstrap_toolchain.py http://gerrit.cloudera.org:8080/#/c/4187/4/bin/save-version.sh File bin/save-version.sh: Line 24 This has been fixed separately in IMPALA-4116 http://gerrit.cloudera.org:8080/#/c/4187/4/bin/start-impala-cluster.py File bin/start-impala-cluster.py: PS4, Line 220: : IMPALA-4208 http://gerrit.cloudera.org:8080/#/c/4187/1/common/thrift/generate_metrics.py File common/thrift/generate_metrics.py: 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 save-version.sh, so the 'cdh' is gone. :) http://gerrit.cloudera.org:8080/#/c/4187/4/common/thrift/generate_metrics.py File common/thrift/generate_metrics.py: 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 save-version.sh. http://gerrit.cloudera.org:8080/#/c/4187/4/ext-data-source/api/pom.xml File ext-data-source/api/pom.xml: IMPALA-4225 http://gerrit.cloudera.org:8080/#/c/4187/4/ext-data-source/sample/pom.xml File ext-data-source/sample/pom.xml: IMPALA-4225 http://gerrit.cloudera.org:8080/#/c/4187/4/ext-data-source/test/pom.xml File ext-data-source/test/pom.xml: IMPALA-4225 http://gerrit.cloudera.org:8080/#/c/4187/4/fe/pom.xml File fe/pom.xml: IMPALA-4225 http://gerrit.cloudera.org:8080/#/c/4187/4/infra/deploy/deploy.py File infra/deploy/deploy.py: PS4, Line 23: I doubt this is accurate, otherwise it could probably be removed, since this quite an old version. @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. http://gerrit.cloudera.org:8080/#/c/4187/3/testdata/cluster/.gitignore 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. http://gerrit.cloudera.org:8080/#/c/4187/1/testdata/cluster/admin File testdata/cluster/admin: > Yes, I think I was wrong. There was a similar discussion in bin/start-impal IMPALA-4208 http://gerrit.cloudera.org:8080/#/c/4187/4/testdata/cluster/admin File testdata/cluster/admin: IMPALA-4208 PS4, Line 28: : IMPALA-4208 http://gerrit.cloudera.org:8080/#/c/4187/4/testdata/datasets/functional/schema_constraints.csv 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 read-only? http://gerrit.cloudera.org:8080/#/c/4187/4/tests/comparison/cluster.py File tests/comparison/cluster.py: IMPALA-4208 http://gerrit.cloudera.org:8080/#/c/4187/3/tests/comparison/leopard/impala_docker_env.py File tests/comparison/leopard/impala_docker_env.py: 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? http://gerrit.cloudera.org:8080/#/c/4187/4/tests/metadata/test_compute_stats.py File tests/metadata/test_compute_stats.py: PS4, Line 45: I reworded this. http://gerrit.cloudera.org:8080/#/c/4187/4/tests/query_test/test_decimal_queries.py File tests/query_test/test_decimal_queries.py: Line 38 This seems to be Hive < 0.11, so I will reword this. http://gerrit.cloudera.org:8080/#/c/4187/4/tests/test-hive-udfs/pom.xml File tests/test-hive-udfs/pom.xml: IMPALA-4225 -- To view, visit http://gerrit.cloudera.org:8080/4187 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icb37e2ef0cd9fa0e581d359c5dd3db7812b7b2c8 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars Volker <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Jim Apple <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Michael Brown <[email protected]> Gerrit-HasComments: Yes
