You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@impala.apache.org by "David Knupp (Code Review)" <ge...@cloudera.org> on 2016/06/03 17:06:16 UTC
[Impala-CR](cdh5-trunk) IMPALA-3491: Use unique database fixture in test shell commandline.py.
David Knupp has uploaded a new change for review.
http://gerrit.cloudera.org:8080/3301
Change subject: IMPALA-3491: Use unique_database fixture in test_shell_commandline.py.
......................................................................
IMPALA-3491: Use unique_database fixture in test_shell_commandline.py.
Before this change, a single test database was created for the entire suite,
and each test was marked to run serially. With the addition of a test fixture
in tests/conftest.py to create a unique database per each individual method,
it's possible now to run the tests in parallel. (The tables required by
individual tests are created via local test fixtures.)
As such, any methods which had been responsible for setting up the test
database were removed. Pytest markers for running tests serially were also
removed, except in cases where interactions from running concurrency would
affect other tests.
Additonal minor changes were made to improve PEP-8 compliance.
The non-serial tests were run in a loop ten times to confirm that there weren't
any unexpected failures.
Change-Id: Icdcb04a99c0907fc1ba56baa2497fafb33b0e34e
---
M tests/shell/test_shell_commandline.py
1 file changed, 93 insertions(+), 90 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala refs/changes/01/3301/1
--
To view, visit http://gerrit.cloudera.org:8080/3301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Icdcb04a99c0907fc1ba56baa2497fafb33b0e34e
Gerrit-PatchSet: 1
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
[Impala-CR](cdh5-trunk) IMPALA-3491: Use unique database fixture in test shell commandline.py.
Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has posted comments on this change.
Change subject: IMPALA-3491: Use unique_database fixture in test_shell_commandline.py.
......................................................................
Patch Set 2:
Note: Patch Set 2 just contains some minor spelling / docstring fixes. No functional changes.
--
To view, visit http://gerrit.cloudera.org:8080/3301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Icdcb04a99c0907fc1ba56baa2497fafb33b0e34e
Gerrit-PatchSet: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: No
[Impala-CR](cdh5-trunk) IMPALA-3491: Use unique database fixture in test shell commandline.py.
Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has uploaded a new patch set (#4).
Change subject: IMPALA-3491: Use unique_database fixture in test_shell_commandline.py.
......................................................................
IMPALA-3491: Use unique_database fixture in test_shell_commandline.py.
Before this change, a single test database was created for the entire suite,
and each test was marked to run serially. With the addition of a test fixture
in tests/conftest.py to create a unique database per each individual method,
it's possible now to run the tests in parallel. (The tables required by
individual tests are created via local test fixtures.)
As such, any methods which had been responsible for setting up the test
database were removed. Pytest markers for running tests serially were also
removed, except in cases where interactions from running concurrency would
affect other tests.
Additional minor changes were made to improve PEP-8 compliance.
The non-serial tests were run in a loop ten times to confirm that there weren't
any unexpected failures.
Change-Id: Icdcb04a99c0907fc1ba56baa2497fafb33b0e34e
---
M tests/shell/test_shell_commandline.py
1 file changed, 100 insertions(+), 91 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala refs/changes/01/3301/4
--
To view, visit http://gerrit.cloudera.org:8080/3301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icdcb04a99c0907fc1ba56baa2497fafb33b0e34e
Gerrit-PatchSet: 4
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Ishaan Joshi <is...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
[Impala-CR](cdh5-trunk) IMPALA-3491: Use unique database fixture in test shell commandline.py.
Posted by "Michael Brown (Code Review)" <ge...@cloudera.org>.
Michael Brown has posted comments on this change.
Change subject: IMPALA-3491: Use unique_database fixture in test_shell_commandline.py.
......................................................................
Patch Set 5: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/3301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Icdcb04a99c0907fc1ba56baa2497fafb33b0e34e
Gerrit-PatchSet: 5
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Ishaan Joshi <is...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: No
[Impala-CR](cdh5-trunk) IMPALA-3491: Use unique database fixture in test shell commandline.py.
Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has posted comments on this change.
Change subject: IMPALA-3491: Use unique_database fixture in test_shell_commandline.py.
......................................................................
Patch Set 4:
(1 comment)
Note that I also added an execute_serially marker back to one test that refreshes the catalog, thereby invalidating the metadata. This was causing intermittent errors for other tests when run concurrently.
http://gerrit.cloudera.org:8080/#/c/3301/3/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:
PS3, Line 56: query_options={'sync_ddl': 1})
> What prefixes?
This is a copy/paste error. I reused your msg from conftest.py. I'll rework it. (That said, in retrospect I'm pretty sure this check is unnecessary here. request.node.name is the same thing as request.function.__name__. You are already confirming that the name + checksum is a valid identifier in your fixture. If that's the case, than the name alone must be valid as well.)
--
To view, visit http://gerrit.cloudera.org:8080/3301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Icdcb04a99c0907fc1ba56baa2497fafb33b0e34e
Gerrit-PatchSet: 4
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Ishaan Joshi <is...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-CR](cdh5-trunk) IMPALA-3491: Use unique database fixture in test shell commandline.py.
Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has posted comments on this change.
Change subject: IMPALA-3491: Use unique_database fixture in test_shell_commandline.py.
......................................................................
Patch Set 5:
Rebase.
--
To view, visit http://gerrit.cloudera.org:8080/3301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Icdcb04a99c0907fc1ba56baa2497fafb33b0e34e
Gerrit-PatchSet: 5
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Ishaan Joshi <is...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: No
[Impala-CR](cdh5-trunk) IMPALA-3491: Use unique database fixture in test shell commandline.py.
Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has posted comments on this change.
Change subject: IMPALA-3491: Use unique_database fixture in test_shell_commandline.py.
......................................................................
Patch Set 6: Code-Review+2
Rebase, no conflicts: propagating +2
--
To view, visit http://gerrit.cloudera.org:8080/3301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Icdcb04a99c0907fc1ba56baa2497fafb33b0e34e
Gerrit-PatchSet: 6
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
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: Ishaan Joshi <is...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: No
[Impala-CR](cdh5-trunk) IMPALA-3491: Use unique database fixture in test shell commandline.py.
Posted by "Michael Brown (Code Review)" <ge...@cloudera.org>.
Michael Brown has posted comments on this change.
Change subject: IMPALA-3491: Use unique_database fixture in test_shell_commandline.py.
......................................................................
Patch Set 2:
(6 comments)
This looks pretty good! Let's set up a private Jenkins build, especially since you're introducing new serial tests, but also to get you introduced to that flow.
http://gerrit.cloudera.org:8080/#/c/3301/2//COMMIT_MSG
Commit Message:
PS2, Line 20: Additonal
Additional
http://gerrit.cloudera.org:8080/#/c/3301/2/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:
PS2, Line 51: fq_table_name = '.'.join([unique_database, request.node.name])
We need to make sure that request.node.name is a valid Impala identifier, because the length constraints of a valid Python identifier are longer than those of Impala. There's already a helper method somewhere in tests.common that tells if you if the string is a valid identifier. You also need to decide how to handle the (unlikely but possible) case that the request.node.name is an invalid Impala identifier. unique_database does this sort of handling, too, so you can look there for example.
PS2, Line 53: request.instance.execute_query(stmt)
We want this to succeed in all cases, and we should fail if it doesn't. There's a execute_query_expect_success() method you can use instead for this purpose.
PS2, Line 74: request.instance.execute_query(stmt)
Same thing here: we want this to succeed.
PS2, Line 78: class TestImpalaShell(ImpalaTestSuite):
Can you double check with Ishaan that making this inherit now is OK? Since you removed teardown_class and setup_class it seems safe, but I want to make sure we didn't miss something here.
PS2, Line 143: @pytest.mark.execute_serially
: def test_kerberos_option(self):
Did you ever figure out why this must be serial?
--
To view, visit http://gerrit.cloudera.org:8080/3301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Icdcb04a99c0907fc1ba56baa2497fafb33b0e34e
Gerrit-PatchSet: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-CR](cdh5-trunk) IMPALA-3491: Use unique database fixture in test shell commandline.py.
Posted by "Michael Brown (Code Review)" <ge...@cloudera.org>.
Michael Brown has posted comments on this change.
Change subject: IMPALA-3491: Use unique_database fixture in test_shell_commandline.py.
......................................................................
Patch Set 5:
I'm a jerk for doing this to your patch set. Apologies!
--
To view, visit http://gerrit.cloudera.org:8080/3301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Icdcb04a99c0907fc1ba56baa2497fafb33b0e34e
Gerrit-PatchSet: 5
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
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: Ishaan Joshi <is...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: No
[Impala-CR](cdh5-trunk) IMPALA-3491: Use unique database fixture in test shell commandline.py.
Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has posted comments on this change.
Change subject: IMPALA-3491: Use unique_database fixture in test_shell_commandline.py.
......................................................................
Patch Set 6: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/3301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Icdcb04a99c0907fc1ba56baa2497fafb33b0e34e
Gerrit-PatchSet: 6
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
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: Ishaan Joshi <is...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: No
[Impala-CR](cdh5-trunk) IMPALA-3491: Use unique database fixture in test shell commandline.py.
Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has uploaded a new patch set (#3).
Change subject: IMPALA-3491: Use unique_database fixture in test_shell_commandline.py.
......................................................................
IMPALA-3491: Use unique_database fixture in test_shell_commandline.py.
Before this change, a single test database was created for the entire suite,
and each test was marked to run serially. With the addition of a test fixture
in tests/conftest.py to create a unique database per each individual method,
it's possible now to run the tests in parallel. (The tables required by
individual tests are created via local test fixtures.)
As such, any methods which had been responsible for setting up the test
database were removed. Pytest markers for running tests serially were also
removed, except in cases where interactions from running concurrency would
affect other tests.
Additonal minor changes were made to improve PEP-8 compliance.
The non-serial tests were run in a loop ten times to confirm that there weren't
any unexpected failures.
Change-Id: Icdcb04a99c0907fc1ba56baa2497fafb33b0e34e
---
M tests/shell/test_shell_commandline.py
1 file changed, 101 insertions(+), 90 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala refs/changes/01/3301/3
--
To view, visit http://gerrit.cloudera.org:8080/3301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icdcb04a99c0907fc1ba56baa2497fafb33b0e34e
Gerrit-PatchSet: 3
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Ishaan Joshi <is...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
[Impala-CR](cdh5-trunk) IMPALA-3491: Use unique database fixture in test shell commandline.py.
Posted by "Michael Brown (Code Review)" <ge...@cloudera.org>.
Michael Brown has posted comments on this change.
Change subject: IMPALA-3491: Use unique_database fixture in test_shell_commandline.py.
......................................................................
Patch Set 2:
(3 comments)
Just a few nits left.
http://gerrit.cloudera.org:8080/#/c/3301/2//COMMIT_MSG
Commit Message:
PS2, Line 20: Additonal
> Additional
This still isn't fixed. Thanks.
http://gerrit.cloudera.org:8080/#/c/3301/3/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:
PS3, Line 55:
Remove "Unique" here. The table name doesn't claim to be unique.
PS3, Line 56:
What prefixes?
--
To view, visit http://gerrit.cloudera.org:8080/3301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Icdcb04a99c0907fc1ba56baa2497fafb33b0e34e
Gerrit-PatchSet: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Ishaan Joshi <is...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-CR](cdh5-trunk) IMPALA-3491: Use unique database fixture in test shell commandline.py.
Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has uploaded a new patch set (#2).
Change subject: IMPALA-3491: Use unique_database fixture in test_shell_commandline.py.
......................................................................
IMPALA-3491: Use unique_database fixture in test_shell_commandline.py.
Before this change, a single test database was created for the entire suite,
and each test was marked to run serially. With the addition of a test fixture
in tests/conftest.py to create a unique database per each individual method,
it's possible now to run the tests in parallel. (The tables required by
individual tests are created via local test fixtures.)
As such, any methods which had been responsible for setting up the test
database were removed. Pytest markers for running tests serially were also
removed, except in cases where interactions from running concurrency would
affect other tests.
Additonal minor changes were made to improve PEP-8 compliance.
The non-serial tests were run in a loop ten times to confirm that there weren't
any unexpected failures.
Change-Id: Icdcb04a99c0907fc1ba56baa2497fafb33b0e34e
---
M tests/shell/test_shell_commandline.py
1 file changed, 94 insertions(+), 90 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala refs/changes/01/3301/2
--
To view, visit http://gerrit.cloudera.org:8080/3301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icdcb04a99c0907fc1ba56baa2497fafb33b0e34e
Gerrit-PatchSet: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
[Impala-CR](cdh5-trunk) IMPALA-3491: Use unique database fixture in test shell commandline.py.
Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has posted comments on this change.
Change subject: IMPALA-3491: Use unique_database fixture in test_shell_commandline.py.
......................................................................
Patch Set 1:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/3301/2//COMMIT_MSG
Commit Message:
PS2, Line 20: Additonal
> Additional
Done
http://gerrit.cloudera.org:8080/#/c/3301/2/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:
PS2, Line 78: """A set of sanity tests for the Impa
> OK, will do. (I should probably add him as a reviewer anyway.)
Done
http://gerrit.cloudera.org:8080/#/c/3301/3/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:
PS3, Line 55:
> Remove "Unique" here. The table name doesn't claim to be unique.
Done
PS3, Line 56: @pytest.fixture
> This is a copy/paste error. I reused your msg from conftest.py. I'll rework
Done
--
To view, visit http://gerrit.cloudera.org:8080/3301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Icdcb04a99c0907fc1ba56baa2497fafb33b0e34e
Gerrit-PatchSet: 1
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Ishaan Joshi <is...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-CR](cdh5-trunk) IMPALA-3491: Use unique database fixture in test shell commandline.py.
Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has posted comments on this change.
Change subject: IMPALA-3491: Use unique_database fixture in test_shell_commandline.py.
......................................................................
Patch Set 5: Verified+1
Passed core/hdfs private build.
http://sandbox.jenkins.cloudera.com/view/Impala/view/Private-Utility/job/impala-private-build-and-test/3346/
--
To view, visit http://gerrit.cloudera.org:8080/3301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Icdcb04a99c0907fc1ba56baa2497fafb33b0e34e
Gerrit-PatchSet: 5
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Ishaan Joshi <is...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: No
[Impala-CR](cdh5-trunk) IMPALA-3491: Use unique database fixture in test shell commandline.py.
Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has posted comments on this change.
Change subject: IMPALA-3491: Use unique_database fixture in test_shell_commandline.py.
......................................................................
Patch Set 5: Verified-1
Build failed: http://sandbox.jenkins.cloudera.com/job/mikeb-gvm/11/
--
To view, visit http://gerrit.cloudera.org:8080/3301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Icdcb04a99c0907fc1ba56baa2497fafb33b0e34e
Gerrit-PatchSet: 5
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
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: Ishaan Joshi <is...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: No
[Impala-CR](cdh5-trunk) IMPALA-3491: Use unique database fixture in test shell commandline.py.
Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
Hello Michael Brown, Internal Jenkins, Alex Behm,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/3301
to look at the new patch set (#6).
Change subject: IMPALA-3491: Use unique_database fixture in test_shell_commandline.py.
......................................................................
IMPALA-3491: Use unique_database fixture in test_shell_commandline.py.
Before this change, a single test database was created for the entire suite,
and each test was marked to run serially. With the addition of a test fixture
in tests/conftest.py to create a unique database per each individual method,
it's possible now to run the tests in parallel. (The tables required by
individual tests are created via local test fixtures.)
As such, any methods which had been responsible for setting up the test
database were removed. Pytest markers for running tests serially were also
removed, except in cases where interactions from running concurrency would
affect other tests.
Additional minor changes were made to improve PEP-8 compliance.
The non-serial tests were run in a loop ten times to confirm that there weren't
any unexpected failures.
Review: https://gerrit.cloudera.org/#/c/3301/
Change-Id: Icdcb04a99c0907fc1ba56baa2497fafb33b0e34e
---
M tests/shell/test_shell_commandline.py
1 file changed, 100 insertions(+), 91 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala refs/changes/01/3301/6
--
To view, visit http://gerrit.cloudera.org:8080/3301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icdcb04a99c0907fc1ba56baa2497fafb33b0e34e
Gerrit-PatchSet: 6
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
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: Ishaan Joshi <is...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
[Impala-CR](cdh5-trunk) IMPALA-3491: Use unique database fixture in test shell commandline.py.
Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has submitted this change and it was merged.
Change subject: IMPALA-3491: Use unique_database fixture in test_shell_commandline.py.
......................................................................
IMPALA-3491: Use unique_database fixture in test_shell_commandline.py.
Before this change, a single test database was created for the entire suite,
and each test was marked to run serially. With the addition of a test fixture
in tests/conftest.py to create a unique database per each individual method,
it's possible now to run the tests in parallel. (The tables required by
individual tests are created via local test fixtures.)
As such, any methods which had been responsible for setting up the test
database were removed. Pytest markers for running tests serially were also
removed, except in cases where interactions from running concurrency would
affect other tests.
Additional minor changes were made to improve PEP-8 compliance.
The non-serial tests were run in a loop ten times to confirm that there weren't
any unexpected failures.
Review: https://gerrit.cloudera.org/#/c/3301/
Change-Id: Icdcb04a99c0907fc1ba56baa2497fafb33b0e34e
Reviewed-on: http://gerrit.cloudera.org:8080/3301
Reviewed-by: David Knupp <dk...@cloudera.com>
Tested-by: Internal Jenkins
---
M tests/shell/test_shell_commandline.py
1 file changed, 100 insertions(+), 91 deletions(-)
Approvals:
David Knupp: Looks good to me, approved
Internal Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/3301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Icdcb04a99c0907fc1ba56baa2497fafb33b0e34e
Gerrit-PatchSet: 7
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
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: Ishaan Joshi <is...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
[Impala-CR](cdh5-trunk) IMPALA-3491: Use unique database fixture in test shell commandline.py.
Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has posted comments on this change.
Change subject: IMPALA-3491: Use unique_database fixture in test_shell_commandline.py.
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/3301/2/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:
PS2, Line 51: """
> Ah, never mind. I see that you meant just the table name.
Done
PS2, Line 143: assert result_rows[0].split(',') == ['i', 's']
:
> Did you ever figure out why this must be serial?
Nope.
--
To view, visit http://gerrit.cloudera.org:8080/3301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Icdcb04a99c0907fc1ba56baa2497fafb33b0e34e
Gerrit-PatchSet: 3
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Ishaan Joshi <is...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-CR](cdh5-trunk) IMPALA-3491: Use unique database fixture in test shell commandline.py.
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change.
Change subject: IMPALA-3491: Use unique_database fixture in test_shell_commandline.py.
......................................................................
Patch Set 5: Code-Review+2
(2 comments)
http://gerrit.cloudera.org:8080/#/c/3301/5/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:
Line 38: @pytest.fixture
thank you for setting a good example here!
Line 54: stmt = "CREATE TABLE if not exists %s (i integer, s string)" % fq_table_name
Remove the "IF NOT EXISTS" part because we don't want to silently ignore unexpected error conditions.
--
To view, visit http://gerrit.cloudera.org:8080/3301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Icdcb04a99c0907fc1ba56baa2497fafb33b0e34e
Gerrit-PatchSet: 5
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
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: Ishaan Joshi <is...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-CR](cdh5-trunk) IMPALA-3491: Use unique database fixture in test shell commandline.py.
Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has posted comments on this change.
Change subject: IMPALA-3491: Use unique_database fixture in test_shell_commandline.py.
......................................................................
Patch Set 2:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/3301/2/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:
PS2, Line 51: fq_table_name = '.'.join([unique_database, request.node.name])
> We need to make sure that request.node.name is a valid Impala identifier, b
It looks like there's a length limit of 127. Is that 127 for the database name, and another 127 for the table name? Or does the fully qualified name ("db.table") need to be under 127? I'm presuming it's the former.
PS2, Line 53: request.instance.execute_query(stmt)
> We want this to succeed in all cases, and we should fail if it doesn't. The
Done
PS2, Line 74: request.instance.execute_query(stmt)
> Same thing here: we want this to succeed.
Done
PS2, Line 78: class TestImpalaShell(ImpalaTestSuite):
> Can you double check with Ishaan that making this inherit now is OK? Since
OK, will do. (I should probably add him as a reviewer anyway.)
--
To view, visit http://gerrit.cloudera.org:8080/3301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Icdcb04a99c0907fc1ba56baa2497fafb33b0e34e
Gerrit-PatchSet: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-CR](cdh5-trunk) IMPALA-3491: Use unique database fixture in test shell commandline.py.
Posted by "David Knupp (Code Review)" <ge...@cloudera.org>.
David Knupp has posted comments on this change.
Change subject: IMPALA-3491: Use unique_database fixture in test_shell_commandline.py.
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/3301/2/tests/shell/test_shell_commandline.py
File tests/shell/test_shell_commandline.py:
PS2, Line 51: fq_table_name = '.'.join([unique_database, request.node.name])
> We need to make sure that request.node.name is a valid Impala identifier, b
Ah, never mind. I see that you meant just the table name.
--
To view, visit http://gerrit.cloudera.org:8080/3301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Icdcb04a99c0907fc1ba56baa2497fafb33b0e34e
Gerrit-PatchSet: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: David Knupp <dk...@cloudera.com>
Gerrit-Reviewer: Ishaan Joshi <is...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: Yes