You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "David Knupp (Code Review)" <ge...@cloudera.org> on 2016/10/14 19:27:39 UTC
[Impala-ASF-CR] IMPALA-4294: Execute data load steps from within git repo.
David Knupp has uploaded a new change for review.
http://gerrit.cloudera.org:8080/4726
Change subject: IMPALA-4294: Execute data load steps from within git repo.
......................................................................
IMPALA-4294: Execute data load steps from within git repo.
Fixes a regression in the data load process that had been introduced
by commit 75a857c. We need to make sure that data load scripts are run
from within the git repo, since they rely upon 'git diff' to determine
if there have been metastore schema changes.
Change-Id: I32e0dce2c10c443763a038aa3b64b1c123ed62ad
---
M buildall.sh
1 file changed, 4 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/4726/2
--
To view, visit http://gerrit.cloudera.org:8080/4726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I32e0dce2c10c443763a038aa3b64b1c123ed62ad
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-4294: Execute data load steps from within git repo.
Posted by "Taras Bobrovytsky (Code Review)" <ge...@cloudera.org>.
Taras Bobrovytsky has posted comments on this change.
Change subject: IMPALA-4294: Execute data load steps from within git repo.
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4726/2/buildall.sh
File buildall.sh:
PS2, Line 431: # Data loading relies upon comparing git hashes, so we need to make sure
: # we're executing from within the context of the Impala repo directory.
: pushd "${IMPALA_HOME}"
: load_test_data
: popd
> Does this work for you? It doesn't seem to work for me. I get different res
Ok, it turns out work tree must be specified as well
git --git-dir ${IMPALA_HOME}/.git --work-tree=${IMPALA_HOME} diff --exit-code a9c4059..HEAD ${IMPALA_HOME}/testdata/datasets/$DATASET
--
To view, visit http://gerrit.cloudera.org:8080/4726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I32e0dce2c10c443763a038aa3b64b1c123ed62ad
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4294: Make check-schema-diff.sh executable from anywhere.
Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has submitted this change and it was merged.
Change subject: IMPALA-4294: Make check-schema-diff.sh executable from anywhere.
......................................................................
IMPALA-4294: Make check-schema-diff.sh executable from anywhere.
Fixes a regression in the data load process that had been introduced
by commit 75a857c. To making check-schema-diff.sh work from anywhere.
we need to specify the git-dir and work-tree arguments everywhere we
call git.
Change-Id: I32e0dce2c10c443763a038aa3b64b1c123ed62ad
Reviewed-on: http://gerrit.cloudera.org:8080/4726
Reviewed-by: Taras Bobrovytsky <tb...@cloudera.com>
Tested-by: Internal Jenkins
---
M testdata/bin/check-schema-diff.sh
1 file changed, 10 insertions(+), 4 deletions(-)
Approvals:
Taras Bobrovytsky: Looks good to me, approved
Internal Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/4726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I32e0dce2c10c443763a038aa3b64b1c123ed62ad
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-4294: Make check-schema-diff.sh executable from anywhere.
Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has posted comments on this change.
Change subject: IMPALA-4294: Make check-schema-diff.sh executable from anywhere.
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4726/2/buildall.sh
File buildall.sh:
PS2, Line 431: load_test_data
: fi
:
: if [[ $TESTS_ACTION -eq 1 ]]; then
: run_
> Ok, it turns out work tree must be specified as well
Done
--
To view, visit http://gerrit.cloudera.org:8080/4726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I32e0dce2c10c443763a038aa3b64b1c123ed62ad
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4294: Execute data load steps from within git repo.
Posted by "Taras Bobrovytsky (Code Review)" <ge...@cloudera.org>.
Taras Bobrovytsky has posted comments on this change.
Change subject: IMPALA-4294: Execute data load steps from within git repo.
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4726/2/buildall.sh
File buildall.sh:
PS2, Line 431: # Data loading relies upon comparing git hashes, so we need to make sure
: # we're executing from within the context of the Impala repo directory.
: pushd "${IMPALA_HOME}"
: load_test_data
: popd
> I like having pushd/popd happen in the same script, rather than pushd in on
Ok, how about adding
git --git-dir $IMPALA_HOME/.git diff --exit-code....
to the check-schema-diff script? We want to make the script stand alone and not rely on an external script to set the directory for it.
--
To view, visit http://gerrit.cloudera.org:8080/4726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I32e0dce2c10c443763a038aa3b64b1c123ed62ad
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4294: Make check-schema-diff.sh executable from anywhere.
Posted by "Taras Bobrovytsky (Code Review)" <ge...@cloudera.org>.
Taras Bobrovytsky has posted comments on this change.
Change subject: IMPALA-4294: Make check-schema-diff.sh executable from anywhere.
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/4726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I32e0dce2c10c443763a038aa3b64b1c123ed62ad
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4294: Execute data load steps from within git repo.
Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has posted comments on this change.
Change subject: IMPALA-4294: Execute data load steps from within git repo.
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4726/2/buildall.sh
File buildall.sh:
PS2, Line 431: # Data loading relies upon comparing git hashes, so we need to make sure
: # we're executing from within the context of the Impala repo directory.
: pushd "${IMPALA_HOME}"
: load_test_data
: popd
> Do you think it would make more sense to pushd in Impala/testdata/bin/check
I like having pushd/popd happen in the same script, rather than pushd in one script, and popd in another script. If I put popd at the end of check-schema-diff.sh, that would affect the exit code (I think, right?) and that's what we check for.
--
To view, visit http://gerrit.cloudera.org:8080/4726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I32e0dce2c10c443763a038aa3b64b1c123ed62ad
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4294: Make check-schema-diff.sh executable from anywhere.
Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has posted comments on this change.
Change subject: IMPALA-4294: Make check-schema-diff.sh executable from anywhere.
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/4726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I32e0dce2c10c443763a038aa3b64b1c123ed62ad
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4294: Execute data load steps from within git repo.
Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has posted comments on this change.
Change subject: IMPALA-4294: Execute data load steps from within git repo.
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4726/2/buildall.sh
File buildall.sh:
PS2, Line 431: # Data loading relies upon comparing git hashes, so we need to make sure
: # we're executing from within the context of the Impala repo directory.
: pushd "${IMPALA_HOME}"
: load_test_data
: popd
> Ok, how about adding
Does this work for you? It doesn't seem to work for me. I get different results based upon where I call the script.
From outside the repo:
~ $ git --git-dir ${IMPALA_HOME}/.git diff --exit-code a9c4059..HEAD ${IMPALA_HOME}/testdata/datasets/$DATASET
~ $ echo $?
0
From inside the repo:
~ $ cd Impala
~/Impala (IMPALA_4294) $ git diff --exit-code a9c4059..HEAD ${IMPALA_HOME}/testdata/datasets/$DATASET
diff --git a/testdata/datasets/functional/schema_constraints.csv b/testdata/datasets/functional/schema_constraints.csv
index a22a432..4c9da6d 100644
--- a/testdata/datasets/functional/schema_constraints.csv
+++ b/testdata/datasets/functional/schema_constraints.csv
@@ -123,7 +123,7 @@ table_name:widerow, constraint:exclude, table_format:hbase/none/none
table_name:nullformat_custom, constraint:exclude, table_format:hbase/none/none
table_name:unsupported_types, constraint:exclude, table_format:hbase/none/none
-# On CDH4, decimal can only be tested on formats Impala can write to (text and parquet)
+# Decimal can only be tested on formats Impala can write to (text and parquet).
# TODO: add Avro once Hive or Impala can write Avro decimals
table_name:decimal_tbl, constraint:restrict_to, table_format:text/none/none
table_name:decimal_tiny, constraint:restrict_to, table_format:text/none/none
~/Impala (IMPALA_4294) $ echo $?
1
--
To view, visit http://gerrit.cloudera.org:8080/4726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I32e0dce2c10c443763a038aa3b64b1c123ed62ad
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4294: Execute data load steps from within git repo.
Posted by "Taras Bobrovytsky (Code Review)" <ge...@cloudera.org>.
Taras Bobrovytsky has posted comments on this change.
Change subject: IMPALA-4294: Execute data load steps from within git repo.
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4726/2/buildall.sh
File buildall.sh:
PS2, Line 431: # Data loading relies upon comparing git hashes, so we need to make sure
: # we're executing from within the context of the Impala repo directory.
: pushd "${IMPALA_HOME}"
: load_test_data
: popd
Do you think it would make more sense to pushd in Impala/testdata/bin/check-schema-diff.sh, because it will be closer to where it's actually needed? Also it would make that script more robust.
--
To view, visit http://gerrit.cloudera.org:8080/4726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I32e0dce2c10c443763a038aa3b64b1c123ed62ad
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4294: Make check-schema-diff.sh executable from anywhere.
Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has uploaded a new patch set (#3).
Change subject: IMPALA-4294: Make check-schema-diff.sh executable from anywhere.
......................................................................
IMPALA-4294: Make check-schema-diff.sh executable from anywhere.
Fixes a regression in the data load process that had been introduced
by commit 75a857c. To making check-schema-diff.sh work from anywhere.
we need to specify the git-dir and work-tree arguments everywhere we
call git.
Change-Id: I32e0dce2c10c443763a038aa3b64b1c123ed62ad
---
M testdata/bin/check-schema-diff.sh
1 file changed, 10 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/4726/3
--
To view, visit http://gerrit.cloudera.org:8080/4726
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I32e0dce2c10c443763a038aa3b64b1c123ed62ad
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>