Project

General

Profile

Actions

Story #21230

closed

Remove usage of global "pip install" in package build/test scripts

Added by Tom Clegg 3 months ago. Updated about 1 month ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
-
Start date:
01/12/2024
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
1.0

Description

Example of obsolete usage in source:build/run-library.sh:

  # Note "pip3 install setuptools" fails on debian12 ("error:
  # externally-managed-environment") even if that requirement is
  # already satisfied, so we parse "pip3 list" output instead to check
  # whether we need to do anything.
  if [[ "$($pip list | grep -P -o '^setuptools\s+\K[0-9]+')" -ge 66 ]]; then
    : # OK, already installed
  elif ! $pip install $DASHQ_UNLESS_DEBUG $CACHE_FLAG -U 'setuptools>=66'; then
    echo "Error, unable to upgrade setuptools with" 
    echo "  $pip install $DASHQ_UNLESS_DEBUG $CACHE_FLAG -U 'setuptools>=66'" 
    exit 1

From #20846#note-32:

the modern way to handle things would be to install Python; python3 -m venv VENVDIR; and then do everything inside the virtualenv, including installing setuptools (and upgrading pip if desired).


Subtasks 1 (0 open1 closed)

Task #21332: Review 21230-no-global-pipResolvedPeter Amstutz01/12/2024

Actions

Related issues

Related to Arvados - Story #20846: Support Ubuntu 22.04 LTSResolvedBrett Smith10/30/2023

Actions
Blocks Arvados - Story #21389: Update arvados/jobs Docker image to Debian 12New

Actions
Blocks Arvados - Bug #21390: Update arvados/dev-jobs Docker image to Debian 12New

Actions
Blocks Arvados - Story #21391: Update arvbox to Debian 12New

Actions
Actions #1

Updated by Tom Clegg 3 months ago

Actions #2

Updated by Tom Clegg 3 months ago

  • Target version changed from To be scheduled to Development 2024-01-17 sprint
Actions #3

Updated by Brett Smith about 2 months ago

Survey of the situation based on git grep -Ee '\bv(irtual|)env\b' -e 'pip[0-9.]*[ _]+install' build/ tools/:

  • build/get-package-version.sh: This is straightforward to translate if we can just build a temporary virtualenv, but that might be a noticeable performance hit. It might be nice to figure out the broader context and see whether it's possible to set up a semi-permanent virtualenv for this script to use.
  • All package-build-dockerfiles: All say RUN /usr/bin/pip3 install 'virtualenv<20' and I don't know why, since they also install virtualenv from a distro package. I suspect this is just cruft that can be removed.
  • build/run-library.sh: Looks straightforward to translate. Most of the code already works out of a virtualenv, so this is just a matter of updating the setup process as discussed. There is a weird thing going on where parts of the code install setuptools>=66 and others install setuptools<45 (with the now-hilarious comment "Get the latest setuptools") so maybe need to dig in a little more to figure out if that version restriction is still relevant.
  • build/run-tests.sh: Already updated, no change needed.
  • tools/arvbox/lib/arvbox/docker/common.sh: The pip_install function is used by the doc and sdk services. This seems easy enough to translate, but it might involve reorganizing things a little bit like doing common virtualenv setup as part of building the Docker image. It would help to know (a) is there a place to put a virtualenv that would match existing filesystem conventions, and (b) what things inside arvbox are expecting to depend on Python things. Do we just install the Python tools for shell use? If so, is it sufficient to update the shell profile to automatically activate that virtualenv?
Actions #4

Updated by Brett Smith about 2 months ago

  • Assigned To set to Brett Smith
Actions #5

Updated by Brett Smith about 2 months ago

Brett Smith wrote in #note-3:

  • build/run-library.sh: Looks straightforward to translate. Most of the code already works out of a virtualenv, so this is just a matter of updating the setup process as discussed. There is a weird thing going on where parts of the code install setuptools>=66 and others install setuptools<45 (with the now-hilarious comment "Get the latest setuptools") so maybe need to dig in a little more to figure out if that version restriction is still relevant.

Half of this is actual Python package building, and I have that updated and working fine as part of #20846.

The other half is building Python 2→3 transitional metapackages. These were added in 2021. I'm inclined to take the position that we no longer need them, and they can be removed for the Arvados 3.0 release. So I'm planning to just excise all this code. I'll do my own survey as part of this, but if anyone knows any other part of the stack that depends on these packages, please let me know.

Actions #6

Updated by Brett Smith about 2 months ago

Brett Smith wrote in #note-5:

Brett Smith wrote in #note-3:

  • build/run-library.sh: Looks straightforward to translate. Most of the code already works out of a virtualenv, so this is just a matter of updating the setup process as discussed. There is a weird thing going on where parts of the code install setuptools>=66 and others install setuptools<45 (with the now-hilarious comment "Get the latest setuptools") so maybe need to dig in a little more to figure out if that version restriction is still relevant.

Half of this is actual Python package building, and I have that updated and working fine as part of #20846.

The other half is building Python 2→3 transitional metapackages. These were added in 2021. I'm inclined to take the position that we no longer need them, and they can be removed for the Arvados 3.0 release. So I'm planning to just excise all this code. I'll do my own survey as part of this, but if anyone knows any other part of the stack that depends on these packages, please let me know.

Both these changes were made in the #20846 branch that just got merged.

Actions #7

Updated by Brett Smith about 1 month ago

21230-no-global-pip @ 7bfde74c810b997a300b42a007b096a30233d8a1

Tests: developer-run-tests: #3997
Remainder rerun: developer-run-tests-remainder: #4209
Build: build-packages-multijob: #3956

  • All agreed upon points are implemented / addressed.
    • All global pip install commands found by the earlier survey have been either removed or adjusted to run inside a virtualenv.
  • Anything not implemented (discovered or discussed during work) has a follow-up story.
    • N/A
  • Code is tested and passing, both automated and manual, what manual testing was done is described
    • In addition to runs above, I also successfully ran arvbox start dev from scratch and confirmed that Python utilities were still available through arvbox ashell.
  • Documentation has been updated.
    • N/A
  • Behaves appropriately at the intended scale (describe intended scale).
    • N/A
  • Considered backwards and forwards compatibility issues between client and server.
    • If somebody was running Python scripts inside arvbox, they may need to adjust those to run with /opt/arvados-py/bin/python3. This seems manageable.
  • Follows our coding standards and GUI style guidelines.
    • Yes
Actions #8

Updated by Brett Smith about 1 month ago

Couple small fixes to arvbox doc rendering added at 83ed86d4d016d345cdadf2b33d140a5c5af59938. Since the Jenkins job for arvbox is broken there's nothing to run, but it passed local testing.

Actions #9

Updated by Brett Smith about 1 month ago

  • Status changed from New to In Progress
Actions #10

Updated by Brett Smith about 1 month ago

Aaand now at 1422c2322339ac5cabdc15f8917d56f6ab501834 to fix an internal tooling PyYAML bug. The PyYAML API changed in 6.0, released in 2021, and all our PyYAML dependencies are unversioned. How I'm the first one hitting this, and why I didn't hit it earlier in testing, is a mystery to me. But whatever, I fixed it.

Actions #11

Updated by Peter Amstutz about 1 month ago

I found another script to be updated: arvados/sdk/cwl/test_with_arvbox.sh

This is used by https://ci.arvados.org/view/CWL/job/arvados-cwl-conformance-tests/

Actions #12

Updated by Brett Smith about 1 month ago

Peter Amstutz wrote in #note-11:

I found another script to be updated: arvados/sdk/cwl/test_with_arvbox.sh

This is used by https://ci.arvados.org/view/CWL/job/arvados-cwl-conformance-tests/

Now at acf0375f2aded6b5f8dbc897abd07e2c9dbd7ce4 with fixes to that and another random test script. Successfully rebuilt arvbox locally. arvados-cwl-conformance-tests: #1540 - This is now failing for the same reason as the arvbox build.

Actions #13

Updated by Brett Smith about 1 month ago

  • Blocks Story #21389: Update arvados/jobs Docker image to Debian 12 added
Actions #14

Updated by Brett Smith about 1 month ago

  • Blocks Bug #21390: Update arvados/dev-jobs Docker image to Debian 12 added
Actions #15

Updated by Brett Smith about 1 month ago

Actions #16

Updated by Peter Amstutz about 1 month ago

  • Target version changed from Development 2024-01-17 sprint to Development 2024-01-31 sprint
Actions #17

Updated by Peter Amstutz about 1 month ago

  • Status changed from In Progress to Resolved

I made some additional commits for #21394 starting from acf0375f and merged both of them, so this is resolved (766d2d7ca8dbb5522a8b7de6409c83fbba4a36ca)

Actions

Also available in: Atom PDF