You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Jim Apple (Code Review)" <ge...@cloudera.org> on 2016/09/29 16:51:07 UTC

[Impala-ASF-CR] Don't assume that AUX exists just because a shell variable is set 

Jim Apple has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/4563

Change subject: Don't assume that AUX exists just because a shell variable is set
......................................................................

Don't assume that AUX exists just because a shell variable is set

This caused breakage of the custom cluster tests -
bin/impala-config.sh sets IMPALA_AUX_TEST_HOME to
$IMPALA_HOME/../Impala-auxiliary-tests, unless it is already set, but
this directory is not guaranteed to
exist. bin/run-custom-cluster-tests.sh was expecting that if
IMPALA_AUX_TEST_HOME is set, then that directory must exist.

While I'm changing bin/run-custom-cluster-tests.sh, add some quotes to
protect against paths with spaces in them.

Change-Id: Ieffb6d52c5f95d3257de8c47a039ccb0d45f0867
---
M tests/run-custom-cluster-tests.sh
1 file changed, 8 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/63/4563/1
-- 
To view, visit http://gerrit.cloudera.org:8080/4563
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ieffb6d52c5f95d3257de8c47a039ccb0d45f0867
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>

[Impala-ASF-CR] Don't assume that AUX exists just because a shell variable is set

Posted by "Taras Bobrovytsky (Code Review)" <ge...@cloudera.org>.
Taras Bobrovytsky has posted comments on this change.

Change subject: Don't assume that AUX exists just because a shell variable is set
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4563/2/tests/run-custom-cluster-tests.sh
File tests/run-custom-cluster-tests.sh:

PS2, Line 40: then
            :     impala-py.test custom_cluster/ authorization/ "${AUX_CUSTOM_DIR}" \
            :                    --junitxml="${RESULTS_DIR}/TEST-impala-custom-cluster.xml" \
            :                    --resultlog="${RESULTS_DIR}/TEST-impala-custom-cluster.log" "$@"
            : else
            :     impala-py.test custom_cluster/ authorization/ \
            :                    --junitxml="${RESULTS_DIR}/TEST-impala-custom-cluster.xml" \
            :                    --resultlog="${RESULTS_DIR}/TEST-impala-custom-cluster.log" "$@"
            : fi
I don't like the code repetition, and it's not clear right away what the difference is between the cases. How about something like:

ARGS=("custom_cluster/ authorization/ ");
if [ -d "${AUX_CUSTOM_DIR}" ] then
  ARGS+=(${AUX_CUSTOM_DIR} )
fi
ARGS+=("--junitxml="${RESULTS_DIR}/TEST-impala-custom-cluster.xml ")
ARGS+=("--resultlog="${RESULTS_DIR}/TEST-impala-custom-cluster.log ")
ARGS+=("$@")
impala-py.test "${ARGS[@]}"


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ieffb6d52c5f95d3257de8c47a039ccb0d45f0867
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3229: Don't assume that AUX exists just because of shell env

Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has posted comments on this change.

Change subject: IMPALA-3229: Don't assume that AUX exists just because of shell env
......................................................................


Patch Set 5: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ieffb6d52c5f95d3257de8c47a039ccb0d45f0867
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3229: Don't assume that AUX exists just because of shell env

Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has posted comments on this change.

Change subject: IMPALA-3229: Don't assume that AUX exists just because of shell env
......................................................................


Patch Set 4: Verified-1

Build failed: http://sandbox.jenkins.cloudera.com/job/impala-external-gerrit-verify-merge-ASF/260/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ieffb6d52c5f95d3257de8c47a039ccb0d45f0867
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3229: Don't assume that AUX exists just because of shell env

Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has posted comments on this change.

Change subject: IMPALA-3229: Don't assume that AUX exists just because of shell env
......................................................................


Patch Set 5:

Build failed: http://sandbox.jenkins.cloudera.com/job/impala-external-gerrit-verify-merge-ASF/264/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ieffb6d52c5f95d3257de8c47a039ccb0d45f0867
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] Don't assume that AUX exists just because a shell variable is set

Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.

Change subject: Don't assume that AUX exists just because a shell variable is set
......................................................................


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ieffb6d52c5f95d3257de8c47a039ccb0d45f0867
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] Don't assume that AUX exists just because a shell variable is set

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Hello Michael Brown, Alex Behm,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/4563

to look at the new patch set (#2).

Change subject: Don't assume that AUX exists just because a shell variable is set
......................................................................

Don't assume that AUX exists just because a shell variable is set

This caused breakage of the custom cluster tests -
bin/impala-config.sh sets IMPALA_AUX_TEST_HOME to
$IMPALA_HOME/../Impala-auxiliary-tests, unless it is already set, but
this directory is not guaranteed to
exist. bin/run-custom-cluster-tests.sh was expecting that if
IMPALA_AUX_TEST_HOME is set, then that directory must exist.

While I'm changing bin/run-custom-cluster-tests.sh, add some quotes to
protect against paths with spaces in them.

Change-Id: Ieffb6d52c5f95d3257de8c47a039ccb0d45f0867
---
M tests/run-custom-cluster-tests.sh
1 file changed, 17 insertions(+), 13 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/63/4563/2
-- 
To view, visit http://gerrit.cloudera.org:8080/4563
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ieffb6d52c5f95d3257de8c47a039ccb0d45f0867
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>

[Impala-ASF-CR] IMPALA-3229: Don't assume that AUX exists just because of shell env

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Hello Michael Brown, Alex Behm,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/4563

to look at the new patch set (#4).

Change subject: IMPALA-3229: Don't assume that AUX exists just because of shell env
......................................................................

IMPALA-3229: Don't assume that AUX exists just because of shell env

This caused breakage of the custom cluster tests -
bin/impala-config.sh sets IMPALA_AUX_TEST_HOME to
$IMPALA_HOME/../Impala-auxiliary-tests, unless it is already set, but
this directory is not guaranteed to
exist. bin/run-custom-cluster-tests.sh was expecting that if
IMPALA_AUX_TEST_HOME is set, then that directory must exist.

While I'm changing bin/run-custom-cluster-tests.sh, add some quotes to
protect against paths with spaces in them.

Change-Id: Ieffb6d52c5f95d3257de8c47a039ccb0d45f0867
---
M tests/run-custom-cluster-tests.sh
1 file changed, 16 insertions(+), 13 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/63/4563/4
-- 
To view, visit http://gerrit.cloudera.org:8080/4563
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ieffb6d52c5f95d3257de8c47a039ccb0d45f0867
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>

[Impala-ASF-CR] Don't assume that AUX exists just because a shell variable is set

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.

Change subject: Don't assume that AUX exists just because a shell variable is set
......................................................................


Patch Set 1:

Testing now. I expect the tests to take a few hours.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ieffb6d52c5f95d3257de8c47a039ccb0d45f0867
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] Don't assume that AUX exists just because a shell variable is set

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.

Change subject: Don't assume that AUX exists just because a shell variable is set
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4563/1/tests/run-custom-cluster-tests.sh
File tests/run-custom-cluster-tests.sh:

PS1, Line 42: TEST-impala-custom-
pytest sees the zero-length argument and tries to use it, eventually indexing into it at location 0, causing an IndexError. PS2 does things differently. I have testing both with and without an aux directory.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ieffb6d52c5f95d3257de8c47a039ccb0d45f0867
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3229: Don't assume that AUX exists just because of shell env

Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.

Change subject: IMPALA-3229: Don't assume that AUX exists just because of shell env
......................................................................


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ieffb6d52c5f95d3257de8c47a039ccb0d45f0867
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3229: Don't assume that AUX exists just because of shell env

Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has posted comments on this change.

Change subject: IMPALA-3229: Don't assume that AUX exists just because of shell env
......................................................................


Patch Set 5: Verified-1

Build failed: http://sandbox.jenkins.cloudera.com/job/impala-external-gerrit-verify-merge-ASF/263/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ieffb6d52c5f95d3257de8c47a039ccb0d45f0867
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-3229: Don't assume that AUX exists just because of shell env

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.

Change subject: IMPALA-3229: Don't assume that AUX exists just because of shell env
......................................................................


Patch Set 5: Code-Review+2

rebase, carry +2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ieffb6d52c5f95d3257de8c47a039ccb0d45f0867
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] Don't assume that AUX exists just because a shell variable is set

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.

Change subject: Don't assume that AUX exists just because a shell variable is set
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4563/2/tests/run-custom-cluster-tests.sh
File tests/run-custom-cluster-tests.sh:

PS2, Line 40: then
            :     impala-py.test custom_cluster/ authorization/ "${AUX_CUSTOM_DIR}" \
            :                    --junitxml="${RESULTS_DIR}/TEST-impala-custom-cluster.xml" \
            :                    --resultlog="${RESULTS_DIR}/TEST-impala-custom-cluster.log" "$@"
            : else
            :     impala-py.test custom_cluster/ authorization/ \
            :                    --junitxml="${RESULTS_DIR}/TEST-impala-custom-cluster.xml" \
            :                    --resultlog="${RESULTS_DIR}/TEST-impala-custom-cluster.log" "$@"
            : fi
> I don't like the code repetition, and it's not clear right away what the di
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ieffb6d52c5f95d3257de8c47a039ccb0d45f0867
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-3229: Don't assume that AUX exists just because of shell env

Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has submitted this change and it was merged.

Change subject: IMPALA-3229: Don't assume that AUX exists just because of shell env
......................................................................


IMPALA-3229: Don't assume that AUX exists just because of shell env

This caused breakage of the custom cluster tests -
bin/impala-config.sh sets IMPALA_AUX_TEST_HOME to
$IMPALA_HOME/../Impala-auxiliary-tests, unless it is already set, but
this directory is not guaranteed to
exist. bin/run-custom-cluster-tests.sh was expecting that if
IMPALA_AUX_TEST_HOME is set, then that directory must exist.

While I'm changing bin/run-custom-cluster-tests.sh, add some quotes to
protect against paths with spaces in them.

Change-Id: Ieffb6d52c5f95d3257de8c47a039ccb0d45f0867
Reviewed-on: http://gerrit.cloudera.org:8080/4563
Reviewed-by: Jim Apple <jb...@cloudera.com>
Tested-by: Internal Jenkins
---
M tests/run-custom-cluster-tests.sh
1 file changed, 16 insertions(+), 13 deletions(-)

Approvals:
  Jim Apple: Looks good to me, approved
  Internal Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ieffb6d52c5f95d3257de8c47a039ccb0d45f0867
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>

[Impala-ASF-CR] Don't assume that AUX exists just because a shell variable is set

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.

Change subject: Don't assume that AUX exists just because a shell variable is set
......................................................................


Patch Set 1:

I tested this when AUX is present; it passed. I'm now testing when AUX is absent.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ieffb6d52c5f95d3257de8c47a039ccb0d45f0867
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] Don't assume that AUX exists just because a shell variable is set

Posted by "Michael Brown (Code Review)" <ge...@cloudera.org>.
Michael Brown has posted comments on this change.

Change subject: Don't assume that AUX exists just because a shell variable is set
......................................................................


Patch Set 1: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ieffb6d52c5f95d3257de8c47a039ccb0d45f0867
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] Don't assume that AUX exists just because a shell variable is set

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Hello Michael Brown, Alex Behm,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/4563

to look at the new patch set (#3).

Change subject: Don't assume that AUX exists just because a shell variable is set
......................................................................

Don't assume that AUX exists just because a shell variable is set

This caused breakage of the custom cluster tests -
bin/impala-config.sh sets IMPALA_AUX_TEST_HOME to
$IMPALA_HOME/../Impala-auxiliary-tests, unless it is already set, but
this directory is not guaranteed to
exist. bin/run-custom-cluster-tests.sh was expecting that if
IMPALA_AUX_TEST_HOME is set, then that directory must exist.

While I'm changing bin/run-custom-cluster-tests.sh, add some quotes to
protect against paths with spaces in them.

Change-Id: Ieffb6d52c5f95d3257de8c47a039ccb0d45f0867
---
M tests/run-custom-cluster-tests.sh
1 file changed, 16 insertions(+), 13 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/63/4563/3
-- 
To view, visit http://gerrit.cloudera.org:8080/4563
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ieffb6d52c5f95d3257de8c47a039ccb0d45f0867
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>