You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/01/05 12:09:45 UTC

[GitHub] [airflow] uranusjr commented on a change in pull request #20678: Modernizes usage of PIP in Airflow images

uranusjr commented on a change in pull request #20678:
URL: https://github.com/apache/airflow/pull/20678#discussion_r778768590



##########
File path: dev/README_RELEASE_PROVIDER_PACKAGES.md
##########
@@ -611,9 +611,9 @@ provider packages. This is especially helpful when you want to test integrations
 additional tools. Below is an example Dockerfile, which installs providers for Google/
 
 ```dockerfile
-FROM apache/airflow:2.0.0
+FROM apache/airflow:2.2.3
 
-RUN pip install --upgrade --user apache-airflow-providers-google==2.0.0.rc1
+RUN pip install  --user apache-airflow-providers-google==2.2.2.rc1

Review comment:
       ```suggestion
   RUN pip install --user apache-airflow-providers-google==2.2.2.rc1
   ```
   
   Nit

##########
File path: scripts/docker/install_airflow.sh
##########
@@ -56,26 +56,26 @@ function install_airflow() {
             pip uninstall apache-airflow --yes
             pip install ${AIRFLOW_INSTALL_EDITABLE_FLAG} \
                 "${AIRFLOW_INSTALLATION_METHOD}[${AIRFLOW_EXTRAS}]${AIRFLOW_VERSION_SPECIFICATION}"
-        fi
+            fi

Review comment:
       This seems unintended?
   
   ```suggestion
           fi
   ```

##########
File path: scripts/docker/install_pip_version.sh
##########
@@ -30,11 +30,13 @@
 . "$( dirname "${BASH_SOURCE[0]}" )/common.sh"
 
 function install_pip_version() {
-    pip install --no-cache-dir --upgrade "pip==${AIRFLOW_PIP_VERSION}" && mkdir -p /root/.local/bin
+    pip install --disable-pip-version-check --no-cache-dir --upgrade "pip==${AIRFLOW_PIP_VERSION}" &&
+        mkdir -p ${HOME}/.local/bin

Review comment:
       Why is the final `mkdir` needed?

##########
File path: scripts/docker/common.sh
##########
@@ -60,3 +59,9 @@ function common::get_constraints_location() {
         AIRFLOW_CONSTRAINTS_LOCATION="${constraints_base}/${AIRFLOW_CONSTRAINTS}-${python_version}.txt"
     fi
 }
+
+function common::show_pip_version_and_location() {
+   echo "PATH=${PATH}"
+   echo "PIP on path: $(which pip)"

Review comment:
       ```suggestion
      echo "pip on path: $(which pip)"
   ```
   
   Consistency?




-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org