You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Riza Suminto (Code Review)" <ge...@cloudera.org> on 2025/02/11 03:52:26 UTC
[Impala-ASF-CR] IMPALA-13747: Use fresh HS2 client for unique database fixture
Riza Suminto has uploaded this change for review. ( http://gerrit.cloudera.org:8080/22471
Change subject: IMPALA-13747: Use fresh HS2 client for unique_database fixture
......................................................................
IMPALA-13747: Use fresh HS2 client for unique_database fixture
Just like IMPALA-13634 bug, unique_database fixture may taint
ImpalaTestSuite.client by setting 'sync_ddl=True' option, but never
cleaning it up for test method usage.
This patch fix unique_database fixture by always creating fresh HS2
client during CREATE DATABASE and DROP DATABASE.
Testing:
- Run and pass metadata/test_ddl.py
Change-Id: Ib503e829552d436035c57b489ffda0d0299f8405
---
M tests/conftest.py
1 file changed, 25 insertions(+), 18 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/71/22471/1
--
To view, visit http://gerrit.cloudera.org:8080/22471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib503e829552d436035c57b489ffda0d0299f8405
Gerrit-Change-Number: 22471
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
[Impala-ASF-CR] IMPALA-13747: Use fresh HS2 client for unique database fixture
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22471 )
Change subject: IMPALA-13747: Use fresh HS2 client for unique_database fixture
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/22471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib503e829552d436035c57b489ffda0d0299f8405
Gerrit-Change-Number: 22471
Gerrit-PatchSet: 4
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Comment-Date: Thu, 13 Feb 2025 15:52:22 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13747: Use fresh HS2 client for unique database fixture
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22471 )
Change subject: IMPALA-13747: Use fresh HS2 client for unique_database fixture
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/18248/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/22471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib503e829552d436035c57b489ffda0d0299f8405
Gerrit-Change-Number: 22471
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Comment-Date: Tue, 11 Feb 2025 04:18:40 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13747: Use fresh HS2 client for unique database fixture
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22471 )
Change subject: IMPALA-13747: Use fresh HS2 client for unique_database fixture
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/18254/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/22471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib503e829552d436035c57b489ffda0d0299f8405
Gerrit-Change-Number: 22471
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Comment-Date: Tue, 11 Feb 2025 16:19:58 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13747: Use fresh HS2 client for unique database fixture
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/22471 )
Change subject: IMPALA-13747: Use fresh HS2 client for unique_database fixture
......................................................................
Patch Set 2: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/22471/2/tests/conftest.py
File tests/conftest.py:
http://gerrit.cloudera.org:8080/#/c/22471/2/tests/conftest.py@405
PS2, Line 405: protocol=HS2
Do you prefer to always use HS2? vector could be also checked for the protocol to use
--
To view, visit http://gerrit.cloudera.org:8080/22471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib503e829552d436035c57b489ffda0d0299f8405
Gerrit-Change-Number: 22471
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Comment-Date: Wed, 12 Feb 2025 17:43:24 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13747: Use fresh HS2 client for unique database fixture
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22471 )
Change subject: IMPALA-13747: Use fresh HS2 client for unique_database fixture
......................................................................
Patch Set 5: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/22471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib503e829552d436035c57b489ffda0d0299f8405
Gerrit-Change-Number: 22471
Gerrit-PatchSet: 5
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Comment-Date: Fri, 14 Feb 2025 01:15:09 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13747: Use fresh HS2 client for unique database fixture
Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/22471 )
Change subject: IMPALA-13747: Use fresh HS2 client for unique_database fixture
......................................................................
Patch Set 3: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/22471/2/tests/conftest.py
File tests/conftest.py:
http://gerrit.cloudera.org:8080/#/c/22471/2/tests/conftest.py@405
PS2, Line 405: protocol=HS2
> Yes. Creating unique_database should be independent from the test vector.
ack
--
To view, visit http://gerrit.cloudera.org:8080/22471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib503e829552d436035c57b489ffda0d0299f8405
Gerrit-Change-Number: 22471
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Comment-Date: Thu, 13 Feb 2025 12:16:28 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13747: Use fresh HS2 client for unique database fixture
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22471 )
Change subject: IMPALA-13747: Use fresh HS2 client for unique_database fixture
......................................................................
Patch Set 4:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/11445/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/22471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib503e829552d436035c57b489ffda0d0299f8405
Gerrit-Change-Number: 22471
Gerrit-PatchSet: 4
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Comment-Date: Thu, 13 Feb 2025 15:52:23 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13747: Use fresh HS2 client for unique database fixture
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/22471 )
Change subject: IMPALA-13747: Use fresh HS2 client for unique_database fixture
......................................................................
IMPALA-13747: Use fresh HS2 client for unique_database fixture
Just like IMPALA-13634 bug, unique_database fixture may taint
ImpalaTestSuite.client by setting 'sync_ddl=True' option and never
cleaning it up for test method usage.
This patch fixes unique_database fixture by always creating fresh HS2
client during CREATE DATABASE and DROP DATABASE.
Testing:
- Caught an assert error at test_set_fallback_db_for_functions
Expected: "default.fn() unknown for database default"
Actual: "functional.fn() unknown for database functional"
In this case, shared ImpalaTestSuite.client may have changed database
via ImpalaTestSuite.run_test_case() or
ImpalaTestSuite.execute_wrapper() in other test method.
Removing unused 'vector' argument somehow fixed the issue.
- For both query_test/test_udfs.py and metadata/test_ddl.py:
- Fixed flake8 issues and remove unused pytest fixture.
- Run and pass the tests in exhaustive exploration.
Change-Id: Ib503e829552d436035c57b489ffda0d0299f8405
Reviewed-on: http://gerrit.cloudera.org:8080/22471
Reviewed-by: Riza Suminto <ri...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M tests/conftest.py
M tests/metadata/test_ddl.py
M tests/query_test/test_udfs.py
3 files changed, 88 insertions(+), 67 deletions(-)
Approvals:
Riza Suminto: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/22471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib503e829552d436035c57b489ffda0d0299f8405
Gerrit-Change-Number: 22471
Gerrit-PatchSet: 6
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
[Impala-ASF-CR] IMPALA-13747: Use fresh HS2 client for unique database fixture
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22471 )
Change subject: IMPALA-13747: Use fresh HS2 client for unique_database fixture
......................................................................
Patch Set 4: Code-Review-1
--
To view, visit http://gerrit.cloudera.org:8080/22471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib503e829552d436035c57b489ffda0d0299f8405
Gerrit-Change-Number: 22471
Gerrit-PatchSet: 4
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Comment-Date: Thu, 13 Feb 2025 16:46:30 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13747: Use fresh HS2 client for unique database fixture
Posted by "Surya Hebbar (Code Review)" <ge...@cloudera.org>.
Surya Hebbar has posted comments on this change. ( http://gerrit.cloudera.org:8080/22471 )
Change subject: IMPALA-13747: Use fresh HS2 client for unique_database fixture
......................................................................
Patch Set 1:
(5 comments)
Thanks for adding me as a reviewer.
Only small nits.
http://gerrit.cloudera.org:8080/#/c/22471/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/22471/1//COMMIT_MSG@10
PS1, Line 10: , but
nit: by
http://gerrit.cloudera.org:8080/#/c/22471/1//COMMIT_MSG@13
PS1, Line 13: fix
nit: fixes
http://gerrit.cloudera.org:8080/#/c/22471/1/tests/conftest.py
File tests/conftest.py:
http://gerrit.cloudera.org:8080/#/c/22471/1/tests/conftest.py@410
PS1, Line 410:
nit: this indentation style seems different compared across the file
maybe aligning with parentheses is better.
http://gerrit.cloudera.org:8080/#/c/22471/1/tests/conftest.py@416
PS1, Line 416: client.set_configuration({'sync_ddl': sync_ddl})
Is it possible to move this above the loop?
http://gerrit.cloudera.org:8080/#/c/22471/1/tests/conftest.py@421
PS1, Line 421:
nit: indentation style is different
--
To view, visit http://gerrit.cloudera.org:8080/22471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib503e829552d436035c57b489ffda0d0299f8405
Gerrit-Change-Number: 22471
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Comment-Date: Tue, 11 Feb 2025 07:53:32 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13747: Use fresh HS2 client for unique database fixture
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22471 )
Change subject: IMPALA-13747: Use fresh HS2 client for unique_database fixture
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/18275/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/22471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib503e829552d436035c57b489ffda0d0299f8405
Gerrit-Change-Number: 22471
Gerrit-PatchSet: 5
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Comment-Date: Thu, 13 Feb 2025 20:15:33 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13747: Use fresh HS2 client for unique database fixture
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22471 )
Change subject: IMPALA-13747: Use fresh HS2 client for unique_database fixture
......................................................................
Patch Set 4: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/22471/4/tests/conftest.py
File tests/conftest.py:
http://gerrit.cloudera.org:8080/#/c/22471/4/tests/conftest.py@a400
PS4, Line 400:
Resetting the default client back to default database turn out to be important for other reason.
We recently add ImpalaTestSuite.setup_method() that will clear client configuration, but it does not do the database reset.
This missing line is the very reason why test_set_fallback_db_for_functions fail, and I can reproduce it ONLY IF I run the TestUdfTargeted with single parallelism.
--
To view, visit http://gerrit.cloudera.org:8080/22471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib503e829552d436035c57b489ffda0d0299f8405
Gerrit-Change-Number: 22471
Gerrit-PatchSet: 4
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Comment-Date: Thu, 13 Feb 2025 16:46:25 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13747: Use fresh HS2 client for unique database fixture
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Daniel Becker, Surya Hebbar, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/22471
to look at the new patch set (#2).
Change subject: IMPALA-13747: Use fresh HS2 client for unique_database fixture
......................................................................
IMPALA-13747: Use fresh HS2 client for unique_database fixture
Just like IMPALA-13634 bug, unique_database fixture may taint
ImpalaTestSuite.client by setting 'sync_ddl=True' option and never
cleaning it up for test method usage.
This patch fixes unique_database fixture by always creating fresh HS2
client during CREATE DATABASE and DROP DATABASE.
Testing:
- Run and pass metadata/test_ddl.py
Change-Id: Ib503e829552d436035c57b489ffda0d0299f8405
---
M tests/conftest.py
1 file changed, 25 insertions(+), 18 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/71/22471/2
--
To view, visit http://gerrit.cloudera.org:8080/22471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib503e829552d436035c57b489ffda0d0299f8405
Gerrit-Change-Number: 22471
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
[Impala-ASF-CR] IMPALA-13747: Use fresh HS2 client for unique database fixture
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Daniel Becker, Surya Hebbar, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/22471
to look at the new patch set (#3).
Change subject: IMPALA-13747: Use fresh HS2 client for unique_database fixture
......................................................................
IMPALA-13747: Use fresh HS2 client for unique_database fixture
Just like IMPALA-13634 bug, unique_database fixture may taint
ImpalaTestSuite.client by setting 'sync_ddl=True' option and never
cleaning it up for test method usage.
This patch fixes unique_database fixture by always creating fresh HS2
client during CREATE DATABASE and DROP DATABASE.
Testing:
- Caught an assert error at test_set_fallback_db_for_functions
Expected: "default.fn() unknown for database default"
Actual: "functional.fn() unknown for database functional"
In this case, shared ImpalaTestSuite.client may have changed database
via ImpalaTestSuite.run_test_case() or
ImpalaTestSuite.execute_wrapper() in other test method.
Removing unused 'vector' argument somehow fixed the issue.
- For both query_test/test_udfs.py and metadata/test_ddl.py:
- Fixed flake8 issues and remove unused pytest fixture.
- Run and pass the tests in exhaustive exploration.
Change-Id: Ib503e829552d436035c57b489ffda0d0299f8405
---
M tests/conftest.py
M tests/metadata/test_ddl.py
M tests/query_test/test_udfs.py
3 files changed, 87 insertions(+), 69 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/71/22471/3
--
To view, visit http://gerrit.cloudera.org:8080/22471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib503e829552d436035c57b489ffda0d0299f8405
Gerrit-Change-Number: 22471
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
[Impala-ASF-CR] IMPALA-13747: Use fresh HS2 client for unique database fixture
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/22471 )
Change subject: IMPALA-13747: Use fresh HS2 client for unique_database fixture
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/22471/1/tests/conftest.py
File tests/conftest.py:
http://gerrit.cloudera.org:8080/#/c/22471/1/tests/conftest.py@410
PS1, Line 410:
> nit: this indentation style seems different compared across the file
In Impala, continuation lines are supposed to add an indentation of +4 spaces, so actually this is the indentation style that conforms to the style guide.
--
To view, visit http://gerrit.cloudera.org:8080/22471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib503e829552d436035c57b489ffda0d0299f8405
Gerrit-Change-Number: 22471
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Comment-Date: Tue, 11 Feb 2025 08:37:19 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13747: Use fresh HS2 client for unique database fixture
Posted by "Surya Hebbar (Code Review)" <ge...@cloudera.org>.
Surya Hebbar has posted comments on this change. ( http://gerrit.cloudera.org:8080/22471 )
Change subject: IMPALA-13747: Use fresh HS2 client for unique_database fixture
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/22471/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/22471/1//COMMIT_MSG@10
PS1, Line 10: and
> 'and' sounds better.
Ack
http://gerrit.cloudera.org:8080/#/c/22471/1/tests/conftest.py
File tests/conftest.py:
http://gerrit.cloudera.org:8080/#/c/22471/1/tests/conftest.py@410
PS1, Line 410: db
> I have this discussion with Csaba before
Ack
http://gerrit.cloudera.org:8080/#/c/22471/1/tests/conftest.py@421
PS1, Line 421:
> Decide to go with this. Neither command below complain about indentation.
Agreed.
Thanks for the info.
With both flake8 setup.cfg and autopep8 ignoring E121, E125-E128 indentation, the only rule seems to be that there should be a minimum level of indentation(2), tried flake8 with just --isolated as well.
https://peps.python.org/pep-0008/#indentation
According to docs also, this is correct, nothing contradicts it.
Then, I guess we are sticking with the minimum level of indentation while migrating to new pep8 style.
--
To view, visit http://gerrit.cloudera.org:8080/22471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib503e829552d436035c57b489ffda0d0299f8405
Gerrit-Change-Number: 22471
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Comment-Date: Wed, 12 Feb 2025 12:46:21 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13747: Use fresh HS2 client for unique database fixture
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22471 )
Change subject: IMPALA-13747: Use fresh HS2 client for unique_database fixture
......................................................................
Patch Set 2:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/22471/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/22471/1//COMMIT_MSG@10
PS1, Line 10: and
> nit: by
'and' sounds better.
http://gerrit.cloudera.org:8080/#/c/22471/1//COMMIT_MSG@13
PS1, Line 13: fix
> nit: fixes
Done
http://gerrit.cloudera.org:8080/#/c/22471/1/tests/conftest.py
File tests/conftest.py:
http://gerrit.cloudera.org:8080/#/c/22471/1/tests/conftest.py@410
PS1, Line 410: db
> Thanks for noting.
I have this discussion with Csaba before
https://gerrit.cloudera.org/c/22358/4/tests/query_test/test_kudu.py#173
Impala code has been around for long, and PEP8 has changed its guidance multiple time.
https://gerrit.cloudera.org/c/22460/
Surprisingly, neither flake8 nor autopep8 complain about this inconsistency. Thus, I will just go with two space indentation.
http://gerrit.cloudera.org:8080/#/c/22471/1/tests/conftest.py@416
PS1, Line 416: r db_name in db_names:
> Is it possible to move this above the loop?
Done
http://gerrit.cloudera.org:8080/#/c/22471/1/tests/conftest.py@421
PS1, Line 421:
> nit: indentation style is different
Decide to go with this. Neither command below complain about indentation.
impala-flake8 tests/conftest.py
autopep8 --ignore E121,E125,E126,E127,E128,W503,E701 --indent-size 2 --max-line-length 90 -i tests/conftest.py
See also
https://github.com/apache/impala/blob/master/setup.cfg
--
To view, visit http://gerrit.cloudera.org:8080/22471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib503e829552d436035c57b489ffda0d0299f8405
Gerrit-Change-Number: 22471
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Comment-Date: Tue, 11 Feb 2025 15:56:09 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13747: Use fresh HS2 client for unique database fixture
Posted by "Surya Hebbar (Code Review)" <ge...@cloudera.org>.
Surya Hebbar has posted comments on this change. ( http://gerrit.cloudera.org:8080/22471 )
Change subject: IMPALA-13747: Use fresh HS2 client for unique_database fixture
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/22471/1/tests/conftest.py
File tests/conftest.py:
http://gerrit.cloudera.org:8080/#/c/22471/1/tests/conftest.py@410
PS1, Line 410:
> In Impala, continuation lines are supposed to add an indentation of +4 spac
Thanks for noting.
I realize this, and was referring to L561, L63, L365-367 to maintain code style consistency across the file.
--
To view, visit http://gerrit.cloudera.org:8080/22471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib503e829552d436035c57b489ffda0d0299f8405
Gerrit-Change-Number: 22471
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Comment-Date: Tue, 11 Feb 2025 12:13:36 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13747: Use fresh HS2 client for unique database fixture
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Daniel Becker, Surya Hebbar, Csaba Ringhofer, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/22471
to look at the new patch set (#5).
Change subject: IMPALA-13747: Use fresh HS2 client for unique_database fixture
......................................................................
IMPALA-13747: Use fresh HS2 client for unique_database fixture
Just like IMPALA-13634 bug, unique_database fixture may taint
ImpalaTestSuite.client by setting 'sync_ddl=True' option and never
cleaning it up for test method usage.
This patch fixes unique_database fixture by always creating fresh HS2
client during CREATE DATABASE and DROP DATABASE.
Testing:
- Caught an assert error at test_set_fallback_db_for_functions
Expected: "default.fn() unknown for database default"
Actual: "functional.fn() unknown for database functional"
In this case, shared ImpalaTestSuite.client may have changed database
via ImpalaTestSuite.run_test_case() or
ImpalaTestSuite.execute_wrapper() in other test method.
Removing unused 'vector' argument somehow fixed the issue.
- For both query_test/test_udfs.py and metadata/test_ddl.py:
- Fixed flake8 issues and remove unused pytest fixture.
- Run and pass the tests in exhaustive exploration.
Change-Id: Ib503e829552d436035c57b489ffda0d0299f8405
---
M tests/conftest.py
M tests/metadata/test_ddl.py
M tests/query_test/test_udfs.py
3 files changed, 88 insertions(+), 67 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/71/22471/5
--
To view, visit http://gerrit.cloudera.org:8080/22471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib503e829552d436035c57b489ffda0d0299f8405
Gerrit-Change-Number: 22471
Gerrit-PatchSet: 5
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
[Impala-ASF-CR] IMPALA-13747: Use fresh HS2 client for unique database fixture
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22471 )
Change subject: IMPALA-13747: Use fresh HS2 client for unique_database fixture
......................................................................
Patch Set 5: Code-Review+2
(1 comment)
Carry +2.
http://gerrit.cloudera.org:8080/#/c/22471/4/tests/conftest.py
File tests/conftest.py:
http://gerrit.cloudera.org:8080/#/c/22471/4/tests/conftest.py@a400
PS4, Line 400:
> Resetting the default client back to default database turn out to be import
I'm putting this back. Will revisit at IMPALA-13758.
--
To view, visit http://gerrit.cloudera.org:8080/22471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib503e829552d436035c57b489ffda0d0299f8405
Gerrit-Change-Number: 22471
Gerrit-PatchSet: 5
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Comment-Date: Thu, 13 Feb 2025 19:50:20 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-13747: Use fresh HS2 client for unique database fixture
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22471 )
Change subject: IMPALA-13747: Use fresh HS2 client for unique_database fixture
......................................................................
Patch Set 5:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/11448/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/22471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib503e829552d436035c57b489ffda0d0299f8405
Gerrit-Change-Number: 22471
Gerrit-PatchSet: 5
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Comment-Date: Thu, 13 Feb 2025 19:50:56 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13747: Use fresh HS2 client for unique database fixture
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/22471 )
Change subject: IMPALA-13747: Use fresh HS2 client for unique_database fixture
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/22471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib503e829552d436035c57b489ffda0d0299f8405
Gerrit-Change-Number: 22471
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Comment-Date: Wed, 12 Feb 2025 13:09:17 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-13747: Use fresh HS2 client for unique database fixture
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22471 )
Change subject: IMPALA-13747: Use fresh HS2 client for unique_database fixture
......................................................................
Patch Set 3:
(1 comment)
I did little cleanup in patch set 3.
See commit message for explanation.
http://gerrit.cloudera.org:8080/#/c/22471/2/tests/conftest.py
File tests/conftest.py:
http://gerrit.cloudera.org:8080/#/c/22471/2/tests/conftest.py@405
PS2, Line 405: protocol=HS2
> Do you prefer to always use HS2? vector could be also checked for the proto
Yes. Creating unique_database should be independent from the test vector.
--
To view, visit http://gerrit.cloudera.org:8080/22471
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib503e829552d436035c57b489ffda0d0299f8405
Gerrit-Change-Number: 22471
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Comment-Date: Wed, 12 Feb 2025 21:46:28 +0000
Gerrit-HasComments: Yes