Code0x58 commented on a change in pull request #3601:
URL: https://github.com/apache/incubator-heron/pull/3601#discussion_r467631046
##########
File path: bazel_configure.py
##########
@@ -413,7 +413,7 @@ def main():
env_map['AUTOMAKE'] = discover_tool('automake', 'Automake', 'AUTOMAKE',
'1.9.6')
env_map['AUTOCONF'] = discover_tool('autoconf', 'Autoconf', 'AUTOCONF',
'2.6.3')
env_map['MAKE'] = discover_tool('make', 'Make', 'MAKE', '3.81')
- env_map['PYTHON3'] = discover_tool('python3', 'Python3', 'PYTHON3', '3.4')
+ env_map['PYTHON3'] = discover_tool('python3', 'Python3', 'PYTHON3', '3.5')
Review comment:
```suggestion
env_map['PYTHON3'] = discover_tool('python3', 'Python3', 'PYTHON3', '3.6')
```
##########
File path: scripts/packages/BUILD
##########
@@ -604,8 +604,9 @@ genrule(
"export RELEASE_FILE_DIR=$$(pwd)",
"export TMP_DIR=$$(mktemp -d -t heronpy.XXXXX)",
"echo $$TMP_DIR",
- "$(location @virtualenv//:virtualenv) --no-download --quiet --clear
$$TMP_DIR/venv",
- "PS1= source $$TMP_DIR/venv/bin/activate",
+ # Create the virtual environment
+ 'python3 -m venv $$TMP_DIR/venv --clear',
Review comment:
:+1:
##########
File path: docker/compile/deprecated/Dockerfile.ubuntu14.04
##########
@@ -33,7 +33,7 @@ RUN apt-get update && apt-get -y install \
libssl-dev \
git \
libtool \
- python3-dev \
Review comment:
python3.6 is the minimum supported version.
As the base OS is over a year past EOL, and there's no python3.6 packages, I
think this should be removed, rather than leaving it in as moving to deprecated
(unless there is a trivial python3.6 install - but I still don't think worth it)
##########
File path: docker/test/Dockerfile.ubuntu18.04
##########
@@ -29,15 +29,17 @@ RUN apt-get update && apt-get -y install \
libcppunit-dev \
patch \
python3-dev \
+ python3-venv \
wget \
zip \
- virtualenv \
unzip \
git \
curl \
tree \
openjdk-11-jdk-headless
+RUN update-alternatives --install /usr/bin/python python /usr/bin/python3 10
Review comment:
is linking `python` to `python3` needed - was it that transient
dependency which needed it?
##########
File path: tools/rules/pex/BUILD
##########
@@ -70,6 +69,7 @@ genrule(
executable = True,
message = "Bootstrapping pex",
output_to_bindir = True,
- tools = ["@virtualenv"],
+ # tools = ["@virtualenv"],
Review comment:
may as well delete these two lines now
##########
File path: docker/compile/deprecated/Dockerfile.ubuntu16.04
##########
@@ -49,6 +49,8 @@ RUN apt-get update && apt-get -y install \
ENV JAVA_HOME /usr/lib/jvm/java-11-openjdk-amd64
+RUN update-alternatives --install /usr/bin/python python /usr/bin/python3 10
Review comment:
this image also has python3.5, so may as well delete this too. I think
if anyone needed an old base OS image, they'd probably be maintaining their own
Dockerfile for that or similar purposes anyway
##########
File path: bazel_configure.py
##########
@@ -413,7 +413,7 @@ def main():
env_map['AUTOMAKE'] = discover_tool('automake', 'Automake', 'AUTOMAKE',
'1.9.6')
env_map['AUTOCONF'] = discover_tool('autoconf', 'Autoconf', 'AUTOCONF',
'2.6.3')
env_map['MAKE'] = discover_tool('make', 'Make', 'MAKE', '3.81')
- env_map['PYTHON3'] = discover_tool('python3', 'Python3', 'PYTHON3', '3.4')
+ env_map['PYTHON3'] = discover_tool('python3', 'Python3', 'PYTHON3', '3.5')
Review comment:
this should probably check that `python -m venv` returns non-zero (the
version will be same as the python interpreter)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
