Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/23319 )
Change subject: IMPALA-14333: [part 1] Make all pytest compatible with Python3 ...................................................................... Patch Set 3: (5 comments) Great work, it would be great to leave Python 2 behind in our infra! http://gerrit.cloudera.org:8080/#/c/23319/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/23319/1//COMMIT_MSG@7 PS1, Line 7: part 1] This patch tries to keep tests working with both Python2 and Python3 - once the default will be Python3, it will be very easy to break Python2 unless we run precommit tests with Python 2. What is you plan for removing Python 2 test support completely? There could be also a follow up Jira to clean up Python 2 related code, e.g search for sys.version_info.major It also doesn't seem an extreme step to me to remove Python 2 test support in this patch - not having to be Python 2 compatible would simplify some code and if exhaustive tests pass, then I don't see a good reason to use Python 2 tests again. http://gerrit.cloudera.org:8080/#/c/23319/1//COMMIT_MSG@17 PS1, Line 17: Set timezone='Europe/Budapest' in test_sort and test_top_n to get : consistent result for reading functional_parquet.complextypes_structs : and functional_orc_def.complextypes_structs. : : Skip testing HBase at test_except and test_partitioned_top_n because : HBase does not return return rows where alltypesagg.tinyint_col is null. : : Add _run_query_with_client() in test_ranger.py to allow reusing single : Impala client for running several queries. Make sure to close the : clients when test is done. Mark several tests in test_ranger.py with : SkipIfFS.hive because they run query through beeline + HiveServer2, but : Ozone and S3 build environment does not start HiveServer2 by default. : I don't get these changes - these seem to change test logic, and I don't know how these are related to Python 2 vs 3 http://gerrit.cloudera.org:8080/#/c/23319/1//COMMIT_MSG@49 PS1, Line 49: test_decimal_queries.py for json file format because table date_tbl and : decimal_tiny is contrained only for json/none/none (functional_json). : Same as for line 17 - how is this related to Python 2 vs 3? http://gerrit.cloudera.org:8080/#/c/23319/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/23319/3//COMMIT_MSG@63 PS3, Line 63: python2 impala-shell Would it be hard to keep python2 impala-shell tests alive? While the Python used for testing only matter for Impala developer, python2 support for impala-shell may have some value for someone. I wouldn't put much effort into it though. http://gerrit.cloudera.org:8080/#/c/23319/3/tests/custom_cluster/test_saml2_sso.py File tests/custom_cluster/test_saml2_sso.py: http://gerrit.cloudera.org:8080/#/c/23319/3/tests/custom_cluster/test_saml2_sso.py@221 PS3, Line 221: body = "SAMLResponse=%s&RelayState=%s" % (authn_resp, relay_state) Where did the base64 encoding go? Btw fyi there is a review in parallel that heavily modifies this file: https://gerrit.cloudera.org/#/c/23237/ -- To view, visit http://gerrit.cloudera.org:8080/23319 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I401a93b6cc7bcd17f41d24e7a310e0c882a550d4 Gerrit-Change-Number: 23319 Gerrit-PatchSet: 3 Gerrit-Owner: Riza Suminto <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Comment-Date: Fri, 22 Aug 2025 13:04:45 +0000 Gerrit-HasComments: Yes
