Skip to content

Conversation

vanastassiou
Copy link
Contributor

@vanastassiou vanastassiou commented Nov 6, 2020

https://github.com/plotly/streambed/issues/14939

The updated get-pip.py was generated as follows:

curl https://bootstrap.pypa.io/get-pip.py -o get-pip.py

Tested as follows:

    • Update dds-vendor Dockerfile to install new pip version
    • Install Dash Enterprise using the above dds-vendor image
    • Deploy a Dash app with a .buildpacks file containing https://github.com/plotly/heroku-buildpack-python.git#update-pip
    • exec into Dash app container and /app/.heroku/python/bin/pip --version

@vanastassiou vanastassiou self-assigned this Nov 6, 2020
@vanastassiou
Copy link
Contributor Author

@gzork please review.

@vanastassiou
Copy link
Contributor Author

@BRONSOLO just wanted to check in with you re the test steps and confirm the container to test the pip version in, and how.

@BRONSOLO
Copy link
Member

BRONSOLO commented Nov 6, 2020

Hey @vanastassiou the testing steps look good but additional changes are needed for this PR in the end as we're vendoring the pip version being used:

  1. We set pip==9.02 in the dds-vendor Docker file so that will need to be bumped to the intended version (e.g., pip==20.2.4) here: https://github.com/plotly/dds-vendor/blob/a8bb3d389903e0db6331bad38476556db9aa21ad/Dockerfile#L5

  2. We will then need to reference this vendored version by changing pip-9.0.2-py2.py3-none-any.whl to pip-20.2.4-py2.py3-none-any.whl here:

    /app/.heroku/python/bin/python "$PIP_DIR/get-pip.py" --no-index "$GET_PIP_PYPI_INDEX_URL/pip/pip-9.0.2-py2.py3-none-any.whl" "$GET_PIP_PYPI_INDEX_URL/setuptools/setuptools-39.0.1-py2.py3-none-any.whl" "$GET_PIP_PYPI_INDEX_URL/wheel/wheel-0.31.1-py2.py3-none-any.whl" &> /dev/null

  3. Lastly, the version that will be used to compare pip against to trigger an upgrade can be updated (e.g., to 20.2.4) here:

    PIP_UPDATE="20.0.2"

Given the change to dds-vendor in 1), the image will need to be updated within the Replicated yaml for testing as well.

@vanastassiou vanastassiou changed the title Update pip from 19.3.1 to 20.2.4 WIP Update pip from 19.3.1 to 20.2.4 Nov 9, 2020
@vanastassiou vanastassiou changed the title WIP Update pip from 19.3.1 to 20.2.4 Update pip from 19.3.1 to 20.2.4 Nov 9, 2020
@vanastassiou vanastassiou requested review from tarzzz and removed request for deepstqte November 9, 2020 23:56
@vanastassiou
Copy link
Contributor Author

@tarzzz please review.

@chriddyp
Copy link
Member

/app/.heroku/python/bin/python "$PIP_DIR/get-pip.py" --no-index "$GET_PIP_PYPI_INDEX_URL/pip/pip-9.0.2-py2.py3-none-any.whl" "$GET_PIP_PYPI_INDEX_URL/setuptools/setuptools-39.0.1-py2.py3-none-any.whl" "$GET_PIP_PYPI_INDEX_URL/wheel/wheel-0.31.1-py2.py3-none-any.whl" &> /dev/null

Hey folks - I was reading through the heroku-buildpack-python CHANGELOG tonight and came across this: heroku/heroku-buildpack-python#1007. If we're updgrading the heroku buildpack too, then maybe the way we upgrade pip needs to change to use pip instead of get-pip.py? I don't know our own codebase at all, but just thought I'd share if helpful.

@vanastassiou
Copy link
Contributor Author

@chriddyp Upstream does opt to use pip instead of get-pip.py, but I've been finding the sync with upstream rather complicated so was planning to split the sync out into a separate issue to ensure that pip was upgraded in a timely manner.

Copy link
Contributor

@tarzzz tarzzz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💃

@tarzzz
Copy link
Contributor

tarzzz commented Nov 10, 2020

@vanastassiou Can you please add a note in https://github.com/plotly/streambed/issues/15131 about Chris' comment so that it's not lost when we merge this PR.?

@vanastassiou vanastassiou merged commit d45cb1b into master Nov 10, 2020
@vanastassiou vanastassiou deleted the update-pip branch November 10, 2020 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants