You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Vincent Tran (Code Review)" <ge...@cloudera.org> on 2018/06/18 17:49:44 UTC
[Impala-ASF-CR] IMPALA-7181: Fix flaky test shell/test shell commandline.py::TestImpalaShell::test socket opening
Vincent Tran has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10747
Change subject: IMPALA-7181: Fix flaky test shell/test_shell_commandline.py::TestImpalaShell::test_socket_opening
......................................................................
IMPALA-7181: Fix flaky test shell/test_shell_commandline.py::TestImpalaShell::test_socket_opening
test_shell_commandline.py::TestImpalaShell::test_socket_opening
uses netcat to listen to an ephmeral port to verify the expected
socket opening behavior of impala-shell.
This port number is fixed to 42000. When this port happens to
be used by another outbound socket, this test will fail.
This change refactors the test to use socket.bind(). The port used
in this test is no longer fixed and will be picked automatically.
This change also adds the proper cleanup logics to the various
subprocess.Popen objects used in the test.
Change-Id: Idd64632ded936d49fc404bcac75588dd7886be44
---
M tests/shell/test_shell_commandline.py
1 file changed, 29 insertions(+), 21 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/10747/1
--
To view, visit http://gerrit.cloudera.org:8080/10747
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Idd64632ded936d49fc404bcac75588dd7886be44
Gerrit-Change-Number: 10747
Gerrit-PatchSet: 1
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
[Impala-ASF-CR] IMPALA-7181: Fix flaky test shell/test shell commandline.py::TestImpalaShell::test socket opening
Posted by "Vincent Tran (Code Review)" <ge...@cloudera.org>.
Hello Philip Zeyliger, Csaba Ringhofer,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10747
to look at the new patch set (#2).
Change subject: IMPALA-7181: Fix flaky test shell/test_shell_commandline.py::TestImpalaShell::test_socket_opening
......................................................................
IMPALA-7181: Fix flaky test shell/test_shell_commandline.py::TestImpalaShell::test_socket_opening
test_shell_commandline.py::TestImpalaShell::test_socket_opening
uses netcat to listen to an ephmeral port to verify the expected
socket opening behavior of impala-shell.
This port number is fixed to 42000. When this port happens to
be used by another outbound socket, this test will fail.
This change refactors the test to use socket.bind(). The port used
in this test is no longer fixed and will be picked automatically.
This change also adds the proper cleanup logics to the various
subprocess.Popen objects used in the test.
Change-Id: Idd64632ded936d49fc404bcac75588dd7886be44
---
M tests/shell/test_shell_commandline.py
1 file changed, 32 insertions(+), 27 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/10747/2
--
To view, visit http://gerrit.cloudera.org:8080/10747
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idd64632ded936d49fc404bcac75588dd7886be44
Gerrit-Change-Number: 10747
Gerrit-PatchSet: 2
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
[Impala-ASF-CR] IMPALA-7181: Fix flaky test shell/test shell commandline.py::TestImpalaShell::test socket opening
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10747 )
Change subject: IMPALA-7181: Fix flaky test shell/test_shell_commandline.py::TestImpalaShell::test_socket_opening
......................................................................
Patch Set 7: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/10747
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd64632ded936d49fc404bcac75588dd7886be44
Gerrit-Change-Number: 10747
Gerrit-PatchSet: 7
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Jun 2018 20:06:33 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7181: Fix flaky test shell/test shell commandline.py::TestImpalaShell::test socket opening
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10747 )
Change subject: IMPALA-7181: Fix flaky test shell/test_shell_commandline.py::TestImpalaShell::test_socket_opening
......................................................................
Patch Set 7:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2707/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/10747
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd64632ded936d49fc404bcac75588dd7886be44
Gerrit-Change-Number: 10747
Gerrit-PatchSet: 7
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Jun 2018 16:49:13 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7181: Fix flaky test shell/test shell commandline.py::TestImpalaShell::test socket opening
Posted by "Vincent Tran (Code Review)" <ge...@cloudera.org>.
Vincent Tran has posted comments on this change. ( http://gerrit.cloudera.org:8080/10747 )
Change subject: IMPALA-7181: Fix flaky test shell/test_shell_commandline.py::TestImpalaShell::test_socket_opening
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/10747/2/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:
http://gerrit.cloudera.org:8080/#/c/10747/2/tests/shell/test_shell_commandline.py@621
PS2, Line 621: accept
> I think that this will wait forever by default if no one connects to it. se
Good point. Done.
http://gerrit.cloudera.org:8080/#/c/10747/2/tests/shell/test_shell_commandline.py@624
PS2, Line 624: impala_shell.kill()
: connection.close()
> These should be moved to a finally block to call them if there is an except
Done
--
To view, visit http://gerrit.cloudera.org:8080/10747
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd64632ded936d49fc404bcac75588dd7886be44
Gerrit-Change-Number: 10747
Gerrit-PatchSet: 2
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Comment-Date: Mon, 18 Jun 2018 20:12:23 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7181: Fix flaky test shell/test shell commandline.py::TestImpalaShell::test socket opening
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10747 )
Change subject: IMPALA-7181: Fix flaky test shell/test_shell_commandline.py::TestImpalaShell::test_socket_opening
......................................................................
IMPALA-7181: Fix flaky test shell/test_shell_commandline.py::TestImpalaShell::test_socket_opening
test_shell_commandline.py::TestImpalaShell::test_socket_opening
uses netcat to listen to an ephemeral port to verify the expected
socket opening behavior of impala-shell.
This port number is fixed to 42000. When this port happens to
be used by another outbound socket, this test will fail.
This change refactors the test to use socket.bind(). The port used
in this test is no longer fixed and will be picked automatically.
This change also adds the proper cleanup logics to the various
subprocess.Popen objects used in the test.
Change-Id: Idd64632ded936d49fc404bcac75588dd7886be44
Reviewed-on: http://gerrit.cloudera.org:8080/10747
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M tests/shell/test_shell_commandline.py
1 file changed, 38 insertions(+), 28 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/10747
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Idd64632ded936d49fc404bcac75588dd7886be44
Gerrit-Change-Number: 10747
Gerrit-PatchSet: 8
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
[Impala-ASF-CR] IMPALA-7181: Fix flaky test shell/test shell commandline.py::TestImpalaShell::test socket opening
Posted by "Vincent Tran (Code Review)" <ge...@cloudera.org>.
Vincent Tran has posted comments on this change. ( http://gerrit.cloudera.org:8080/10747 )
Change subject: IMPALA-7181: Fix flaky test shell/test_shell_commandline.py::TestImpalaShell::test_socket_opening
......................................................................
Patch Set 4:
Fixed a mistake the test function's block comment.
--
To view, visit http://gerrit.cloudera.org:8080/10747
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd64632ded936d49fc404bcac75588dd7886be44
Gerrit-Change-Number: 10747
Gerrit-PatchSet: 4
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Comment-Date: Mon, 18 Jun 2018 20:18:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7181: Fix flaky test shell/test shell commandline.py::TestImpalaShell::test socket opening
Posted by "Vincent Tran (Code Review)" <ge...@cloudera.org>.
Hello Philip Zeyliger, Csaba Ringhofer,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10747
to look at the new patch set (#4).
Change subject: IMPALA-7181: Fix flaky test shell/test_shell_commandline.py::TestImpalaShell::test_socket_opening
......................................................................
IMPALA-7181: Fix flaky test shell/test_shell_commandline.py::TestImpalaShell::test_socket_opening
test_shell_commandline.py::TestImpalaShell::test_socket_opening
uses netcat to listen to an ephmeral port to verify the expected
socket opening behavior of impala-shell.
This port number is fixed to 42000. When this port happens to
be used by another outbound socket, this test will fail.
This change refactors the test to use socket.bind(). The port used
in this test is no longer fixed and will be picked automatically.
This change also adds the proper cleanup logics to the various
subprocess.Popen objects used in the test.
Change-Id: Idd64632ded936d49fc404bcac75588dd7886be44
---
M tests/shell/test_shell_commandline.py
1 file changed, 36 insertions(+), 28 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/10747/4
--
To view, visit http://gerrit.cloudera.org:8080/10747
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idd64632ded936d49fc404bcac75588dd7886be44
Gerrit-Change-Number: 10747
Gerrit-PatchSet: 4
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
[Impala-ASF-CR] IMPALA-7181: Fix flaky test shell/test shell commandline.py::TestImpalaShell::test socket opening
Posted by "Vincent Tran (Code Review)" <ge...@cloudera.org>.
Vincent Tran has posted comments on this change. ( http://gerrit.cloudera.org:8080/10747 )
Change subject: IMPALA-7181: Fix flaky test shell/test_shell_commandline.py::TestImpalaShell::test_socket_opening
......................................................................
Patch Set 6:
(2 comments)
Good catch.
http://gerrit.cloudera.org:8080/#/c/10747/3/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:
http://gerrit.cloudera.org:8080/#/c/10747/3/tests/shell/test_shell_commandline.py@626
PS3, Line 626: if impala_shell.poll() is None:
> This can actually also throw an exception, if the process already exited fo
Done
http://gerrit.cloudera.org:8080/#/c/10747/3/tests/shell/test_shell_commandline.py@627
PS3, Line 627: impala_shell.kill()
> connection should be None checked.
Done
--
To view, visit http://gerrit.cloudera.org:8080/10747
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd64632ded936d49fc404bcac75588dd7886be44
Gerrit-Change-Number: 10747
Gerrit-PatchSet: 6
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Comment-Date: Mon, 18 Jun 2018 22:16:56 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7181: Fix flaky test shell/test shell commandline.py::TestImpalaShell::test socket opening
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/10747 )
Change subject: IMPALA-7181: Fix flaky test shell/test_shell_commandline.py::TestImpalaShell::test_socket_opening
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/10747/1/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:
http://gerrit.cloudera.org:8080/#/c/10747/1/tests/shell/test_shell_commandline.py@629
PS1, Line 629: # Verify that impala-shell tries to create a socket against the host:port
: # combination specified by -i when -b is not used
: impala_shell = Popen(shlex.split("%s %s" % (shell_cmd, args1, )), stdout=devnull,
: stderr=devnull)
: connection, client_address = s.accept()
: data = connection.recv(1024)
: assert expected_output in data
: impala_shell.kill()
: connection.close()
:
: # Verify that impala-shell tries to create a socket against the host:port
: # combination specified by -i when -b is used
I would prefer if the duplicated logic in the two cases would be merged to a function somehow, but this is optional.
http://gerrit.cloudera.org:8080/#/c/10747/1/tests/shell/test_shell_commandline.py@645
PS1, Line 645: expected_output = "PingImpalaService"
Already set in line 627, no need to set here again.
http://gerrit.cloudera.org:8080/#/c/10747/1/tests/shell/test_shell_commandline.py@649
PS1, Line 649: s.close()
I am a bit worried about exceptions - this could be moved to a finally block or the socket could be added to a with statement (see the python 2 version in https://stackoverflow.com/a/16772515 )
--
To view, visit http://gerrit.cloudera.org:8080/10747
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd64632ded936d49fc404bcac75588dd7886be44
Gerrit-Change-Number: 10747
Gerrit-PatchSet: 1
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Mon, 18 Jun 2018 18:18:18 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7181: Fix flaky test shell/test shell commandline.py::TestImpalaShell::test socket opening
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/10747 )
Change subject: IMPALA-7181: Fix flaky test shell/test_shell_commandline.py::TestImpalaShell::test_socket_opening
......................................................................
Patch Set 2:
(2 comments)
Some socketing concerns, sorry for not noticing them in the first run.
http://gerrit.cloudera.org:8080/#/c/10747/2/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:
http://gerrit.cloudera.org:8080/#/c/10747/2/tests/shell/test_shell_commandline.py@621
PS2, Line 621: accept
I think that this will wait forever by default if no one connects to it. setdefaulttimeout should be to set avoid this ("select" or something similar can be used too, but that will be more complex I guess).
http://gerrit.cloudera.org:8080/#/c/10747/2/tests/shell/test_shell_commandline.py@624
PS2, Line 624: impala_shell.kill()
: connection.close()
These should be moved to a finally block to call them if there is an exception, for example from the assert.
--
To view, visit http://gerrit.cloudera.org:8080/10747
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd64632ded936d49fc404bcac75588dd7886be44
Gerrit-Change-Number: 10747
Gerrit-PatchSet: 2
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Comment-Date: Mon, 18 Jun 2018 19:52:34 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7181: Fix flaky test shell/test shell commandline.py::TestImpalaShell::test socket opening
Posted by "Vincent Tran (Code Review)" <ge...@cloudera.org>.
Hello Philip Zeyliger, Csaba Ringhofer,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10747
to look at the new patch set (#3).
Change subject: IMPALA-7181: Fix flaky test shell/test_shell_commandline.py::TestImpalaShell::test_socket_opening
......................................................................
IMPALA-7181: Fix flaky test shell/test_shell_commandline.py::TestImpalaShell::test_socket_opening
test_shell_commandline.py::TestImpalaShell::test_socket_opening
uses netcat to listen to an ephmeral port to verify the expected
socket opening behavior of impala-shell.
This port number is fixed to 42000. When this port happens to
be used by another outbound socket, this test will fail.
This change refactors the test to use socket.bind(). The port used
in this test is no longer fixed and will be picked automatically.
This change also adds the proper cleanup logics to the various
subprocess.Popen objects used in the test.
Change-Id: Idd64632ded936d49fc404bcac75588dd7886be44
---
M tests/shell/test_shell_commandline.py
1 file changed, 35 insertions(+), 27 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/10747/3
--
To view, visit http://gerrit.cloudera.org:8080/10747
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idd64632ded936d49fc404bcac75588dd7886be44
Gerrit-Change-Number: 10747
Gerrit-PatchSet: 3
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
[Impala-ASF-CR] IMPALA-7181: Fix flaky test shell/test shell commandline.py::TestImpalaShell::test socket opening
Posted by "Vincent Tran (Code Review)" <ge...@cloudera.org>.
Hello Philip Zeyliger, Csaba Ringhofer,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10747
to look at the new patch set (#5).
Change subject: IMPALA-7181: Fix flaky test shell/test_shell_commandline.py::TestImpalaShell::test_socket_opening
......................................................................
IMPALA-7181: Fix flaky test shell/test_shell_commandline.py::TestImpalaShell::test_socket_opening
test_shell_commandline.py::TestImpalaShell::test_socket_opening
uses netcat to listen to an ephemeral port to verify the expected
socket opening behavior of impala-shell.
This port number is fixed to 42000. When this port happens to
be used by another outbound socket, this test will fail.
This change refactors the test to use socket.bind(). The port used
in this test is no longer fixed and will be picked automatically.
This change also adds the proper cleanup logics to the various
subprocess.Popen objects used in the test.
Change-Id: Idd64632ded936d49fc404bcac75588dd7886be44
---
M tests/shell/test_shell_commandline.py
1 file changed, 36 insertions(+), 28 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/10747/5
--
To view, visit http://gerrit.cloudera.org:8080/10747
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idd64632ded936d49fc404bcac75588dd7886be44
Gerrit-Change-Number: 10747
Gerrit-PatchSet: 5
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
[Impala-ASF-CR] IMPALA-7181: Fix flaky test shell/test shell commandline.py::TestImpalaShell::test socket opening
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/10747 )
Change subject: IMPALA-7181: Fix flaky test shell/test_shell_commandline.py::TestImpalaShell::test_socket_opening
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/10747/3/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:
http://gerrit.cloudera.org:8080/#/c/10747/3/tests/shell/test_shell_commandline.py@626
PS3, Line 626: impala_shell.kill()
This can actually also throw an exception, if the process already exited for some reason.
http://gerrit.cloudera.org:8080/#/c/10747/3/tests/shell/test_shell_commandline.py@627
PS3, Line 627: connection.close()
connection should be None checked.
--
To view, visit http://gerrit.cloudera.org:8080/10747
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd64632ded936d49fc404bcac75588dd7886be44
Gerrit-Change-Number: 10747
Gerrit-PatchSet: 3
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Comment-Date: Mon, 18 Jun 2018 20:59:49 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7181: Fix flaky test shell/test shell commandline.py::TestImpalaShell::test socket opening
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10747 )
Change subject: IMPALA-7181: Fix flaky test shell/test_shell_commandline.py::TestImpalaShell::test_socket_opening
......................................................................
Patch Set 6:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2705/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/10747
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd64632ded936d49fc404bcac75588dd7886be44
Gerrit-Change-Number: 10747
Gerrit-PatchSet: 6
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Jun 2018 13:17:10 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7181: Fix flaky test shell/test shell commandline.py::TestImpalaShell::test socket opening
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10747 )
Change subject: IMPALA-7181: Fix flaky test shell/test_shell_commandline.py::TestImpalaShell::test_socket_opening
......................................................................
Patch Set 6: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/10747
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd64632ded936d49fc404bcac75588dd7886be44
Gerrit-Change-Number: 10747
Gerrit-PatchSet: 6
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Jun 2018 16:40:42 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7181: Fix flaky test shell/test shell commandline.py::TestImpalaShell::test socket opening
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/10747 )
Change subject: IMPALA-7181: Fix flaky test shell/test_shell_commandline.py::TestImpalaShell::test_socket_opening
......................................................................
Patch Set 6: Code-Review+2
It turned out that I do not have an account on Jenkins yet, so I can only run DRY_RUN=true at the moment.
--
To view, visit http://gerrit.cloudera.org:8080/10747
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd64632ded936d49fc404bcac75588dd7886be44
Gerrit-Change-Number: 10747
Gerrit-PatchSet: 6
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Jun 2018 13:18:46 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7181: Fix flaky test shell/test shell commandline.py::TestImpalaShell::test socket opening
Posted by "Vincent Tran (Code Review)" <ge...@cloudera.org>.
Hello Philip Zeyliger, Csaba Ringhofer,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10747
to look at the new patch set (#6).
Change subject: IMPALA-7181: Fix flaky test shell/test_shell_commandline.py::TestImpalaShell::test_socket_opening
......................................................................
IMPALA-7181: Fix flaky test shell/test_shell_commandline.py::TestImpalaShell::test_socket_opening
test_shell_commandline.py::TestImpalaShell::test_socket_opening
uses netcat to listen to an ephemeral port to verify the expected
socket opening behavior of impala-shell.
This port number is fixed to 42000. When this port happens to
be used by another outbound socket, this test will fail.
This change refactors the test to use socket.bind(). The port used
in this test is no longer fixed and will be picked automatically.
This change also adds the proper cleanup logics to the various
subprocess.Popen objects used in the test.
Change-Id: Idd64632ded936d49fc404bcac75588dd7886be44
---
M tests/shell/test_shell_commandline.py
1 file changed, 38 insertions(+), 28 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/10747/6
--
To view, visit http://gerrit.cloudera.org:8080/10747
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idd64632ded936d49fc404bcac75588dd7886be44
Gerrit-Change-Number: 10747
Gerrit-PatchSet: 6
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
[Impala-ASF-CR] IMPALA-7181: Fix flaky test shell/test shell commandline.py::TestImpalaShell::test socket opening
Posted by "Vincent Tran (Code Review)" <ge...@cloudera.org>.
Vincent Tran has posted comments on this change. ( http://gerrit.cloudera.org:8080/10747 )
Change subject: IMPALA-7181: Fix flaky test shell/test_shell_commandline.py::TestImpalaShell::test_socket_opening
......................................................................
Patch Set 1:
(3 comments)
Thanks for reviewing!
http://gerrit.cloudera.org:8080/#/c/10747/1/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:
http://gerrit.cloudera.org:8080/#/c/10747/1/tests/shell/test_shell_commandline.py@629
PS1, Line 629: # Verify that impala-shell tries to create a socket against the host:port
: # combination specified by -i when -b is not used
: impala_shell = Popen(shlex.split("%s %s" % (shell_cmd, args1, )), stdout=devnull,
: stderr=devnull)
: connection, client_address = s.accept()
: data = connection.recv(1024)
: assert expected_output in data
: impala_shell.kill()
: connection.close()
:
: # Verify that impala-shell tries to create a socket against the host:port
: # combination specified by -i when -b is used
> I would prefer if the duplicated logic in the two cases would be merged to
Good idea. Done.
http://gerrit.cloudera.org:8080/#/c/10747/1/tests/shell/test_shell_commandline.py@645
PS1, Line 645: expected_output = "PingImpalaService"
> Already set in line 627, no need to set here again.
Yep. Thought I cleaned this up but it snuck back in after a few edits.
http://gerrit.cloudera.org:8080/#/c/10747/1/tests/shell/test_shell_commandline.py@649
PS1, Line 649: s.close()
> I am a bit worried about exceptions - this could be moved to a finally bloc
Done
--
To view, visit http://gerrit.cloudera.org:8080/10747
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd64632ded936d49fc404bcac75588dd7886be44
Gerrit-Change-Number: 10747
Gerrit-PatchSet: 1
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Comment-Date: Mon, 18 Jun 2018 19:05:59 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7181: Fix flaky test shell/test shell commandline.py::TestImpalaShell::test socket opening
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10747 )
Change subject: IMPALA-7181: Fix flaky test shell/test_shell_commandline.py::TestImpalaShell::test_socket_opening
......................................................................
Patch Set 7: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/10747
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd64632ded936d49fc404bcac75588dd7886be44
Gerrit-Change-Number: 10747
Gerrit-PatchSet: 7
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Jun 2018 16:49:12 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7181: Fix flaky test shell/test shell commandline.py::TestImpalaShell::test socket opening
Posted by "Philip Zeyliger (Code Review)" <ge...@cloudera.org>.
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/10747 )
Change subject: IMPALA-7181: Fix flaky test shell/test_shell_commandline.py::TestImpalaShell::test_socket_opening
......................................................................
Patch Set 6: Code-Review+1
Thanks.
Csaba--do you have any more comments? Feel free to GVD once you're happy with it.
--
To view, visit http://gerrit.cloudera.org:8080/10747
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd64632ded936d49fc404bcac75588dd7886be44
Gerrit-Change-Number: 10747
Gerrit-PatchSet: 6
Gerrit-Owner: Vincent Tran <vt...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Comment-Date: Mon, 18 Jun 2018 23:25:07 +0000
Gerrit-HasComments: No