This is an automated email from the ASF dual-hosted git repository.
michaelsmith pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
The following commit(s) were added to refs/heads/master by this push:
new 403519def IMPALA-13597: Upgrade critique-gerrit-review.py to Python3
403519def is described below
commit 403519def4a2a38fb2a12be68254037976f05a11
Author: Laszlo Gaal <[email protected]>
AuthorDate: Tue Dec 10 15:23:56 2024 +0100
IMPALA-13597: Upgrade critique-gerrit-review.py to Python3
Commit 8e71f5ec8609cc046cf35eb044d91bf34ae9f9c7 has changed the Python
environment for the gerrit-auto-critic script from Python2 to Python3.
Unfortunately the change missed a few Python3-related updates, so the
script started failing in the pre-commit environment.
This patch adds the following updates to the Python3 update:
- changes the virtualenv implementation from virtualenv to the venv
module offered by default in Python3.
- adds pip3 and system_site_packages=True to the venv creation
- bumps the flake8 module to a newer version, as it doesn't have to be
compatible with Python2 any longer.
- extends Popen calls with universal_newlines=True wherever these were
missing.
The patch also fixes a regex search string in test_kudu.py (changes the
regex pattern string to a raw Python string). This is somewhat unrelated
to the Python script change, but it was discovered during testing to
make flake8 emit a badly formatted warning message.
The python3-venv and python3-wheel packages were installed manually on
jenkins.impala.io during testing. These were necessary to eliminate
errors during the scripts initial virtualenv-setup steps.
Tests:
- ran the new script locally
- ran the new script through the precommit process using a test copy of
the gerrit-auto-critic job, test-gerrit-auto-critic.
Change-Id: I5efa035fae38bd42cc3b07f479da2b3983f68252
Reviewed-on: http://gerrit.cloudera.org:8080/22191
Reviewed-by: Riza Suminto <[email protected]>
Reviewed-by: Michael Smith <[email protected]>
Tested-by: Michael Smith <[email protected]>
---
bin/jenkins/critique-gerrit-review.py | 18 ++++++++++--------
tests/query_test/test_kudu.py | 2 +-
2 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/bin/jenkins/critique-gerrit-review.py
b/bin/jenkins/critique-gerrit-review.py
index e0df83228..ac77fe973 100755
--- a/bin/jenkins/critique-gerrit-review.py
+++ b/bin/jenkins/critique-gerrit-review.py
@@ -31,7 +31,7 @@
# Ref: https://gerrit-review.googlesource.com/Documentation/cmd-review.html
#
# Dependencies:
-# ssh, pip, virtualenv
+# ssh, pip, venv
#
# TODO: generalise to other warnings
# * clang-tidy
@@ -44,14 +44,14 @@ from os import environ
import os.path
import re
from subprocess import check_call, check_output, DEVNULL, Popen, PIPE
-import virtualenv
+import venv
-FLAKE8_VERSION = "3.9.2"
+FLAKE8_VERSION = "7.1.1"
FLAKE8_DIFF_VERSION = "0.2.2"
VENV_PATH = "gerrit_critic_venv"
VENV_BIN = os.path.join(VENV_PATH, "bin")
-PIP_PATH = os.path.join(VENV_BIN, "pip")
+PIP_PATH = os.path.join(VENV_BIN, "pip3")
FLAKE8_DIFF_PATH = os.path.join(VENV_BIN, "flake8-diff")
# Limit on length of lines in source files.
@@ -123,8 +123,9 @@ THRIFT_WARNING_SUFFIX = (" might break the compatibility
between impalad and "
def setup_virtualenv():
"""Set up virtualenv with flake8-diff."""
- virtualenv.cli_run([VENV_PATH])
+ venv.create(VENV_PATH, with_pip=True, system_site_packages=True)
check_call([PIP_PATH, "install",
+ "wheel",
"flake8=={0}".format(FLAKE8_VERSION),
"flake8-diff=={0}".format(FLAKE8_DIFF_VERSION)])
@@ -150,7 +151,7 @@ def get_flake8_comments(base_revision, revision):
flake8_diff_proc = Popen(
[FLAKE8_DIFF_PATH, "--standard-flake8-output", "--color", "off",
base_revision,
revision],
- stdin=PIPE, stdout=PIPE, stderr=PIPE, env=flake8_env)
+ stdin=PIPE, stdout=PIPE, stderr=PIPE, env=flake8_env,
universal_newlines=True)
stdout, stderr = flake8_diff_proc.communicate()
# Ignore the return code since it will be non-zero if any violations are
found. We want
# to continue in that case. Instead check stderr for any errors.
@@ -406,7 +407,8 @@ def post_review_to_gerrit(review_input):
proc = Popen(["ssh", "-p", environ["GERRIT_PORT"],
"impala-public-jenkins@" + environ["GERRIT_HOST"], "gerrit",
"review",
"--project", environ["GERRIT_PROJECT"], "--json",
- "{0},{1}".format(change_num, patch_num)], stdin=PIPE)
+ "{0},{1}".format(change_num, patch_num)], stdin=PIPE,
+ universal_newlines=True)
proc.communicate(json.dumps(review_input))
if proc.returncode != 0:
raise Exception("Error posting review to gerrit.")
@@ -445,7 +447,7 @@ if __name__ == "__main__":
review_input["message"] = (
"gerrit-auto-critic failed. You can reproduce it locally using
command:\n\n"
" python3 bin/jenkins/critique-gerrit-review.py --dryrun\n\n"
- "To run it, you might need a virtual env with virtualenv installed.")
+ "To run it, you might need a virtual env with Python3's venv
installed.")
print(json.dumps(review_input, indent=True))
if not args.dryrun:
post_review_to_gerrit(review_input)
diff --git a/tests/query_test/test_kudu.py b/tests/query_test/test_kudu.py
index 32b9ff0f4..21f8a0971 100644
--- a/tests/query_test/test_kudu.py
+++ b/tests/query_test/test_kudu.py
@@ -659,7 +659,7 @@ class TestKuduPartitioning(KuduTestSuite):
# the number of rows.
res = []
kudu_table_sink = "KuduTableSink"
- total_num_rows_re = re.compile("TotalNumRows:.*\(([0-9]+)\)")
+ total_num_rows_re = re.compile(r"TotalNumRows:.*\(([0-9]+)\)")
within_kudu_table_sink_section = False
for line in profile.splitlines():
