Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: [python] Introduce packages option #768

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

alexander-smolyakov
Copy link
Contributor

General info

Feature name:

  • ghcr.io/devcontainers/features/python

Attached related issue:

Description:

This PR introduces a new option - packages. The new option receives a comma-spread string as input. The input contains packages that should be installed during the feature installation process.

User experience

The user can specify a package version that would be installed. Usage examples:

Example 1:

{
        "ghcr.io/devcontainers/features/python:1": {
            "version": "3.10.12",
            "packages": "torch,request,aiohttp"
        },
}

Note: In this example, the feature installs the latest versions available in PIP.

Example 2:

{
        "ghcr.io/devcontainers/features/python:1": {
            "version": "3.10.12",
            "packages": "cryptography==41.0.4,urllib3==1.26.18"
        },
}

Note: In this case, the feature install versions were explicitly pointed out in the devcontainer.json.

Installation process

The feature already contains the install_user_package function. Considering new functionality, the function should be updated, and the installation process should be split into two steps:

  1. Uninstall the older package version if it was installed or shipped with Python;
  2. Install the new package version;

The installation folder will be resolved based on the user under which the feature is installed:

  • root: packages will be installed in Python's site-packages folder;
  • non-root users: packages will be installed in the user's site-packages folder;

Changelog

  • Added info about packages and additionalVersions inputs;

  • Introduced the packages option:

    • Added logic to handle packages input;
    • Implemented the install_python_package function to replace the install_user_package function;
    • The sudo_if function moved to the utils.sh file;
  • Added tests to cover new functionality;

Checklist:

  • Checked that applied changes work as expected

- Create `utils.sh` files;
- Move `sudo_if` to utils.sh`;
- Replace `install_user_package` with `install_python_package` and move function to utils.sh`;
- Add logic to handle `packages` input;
@alexander-smolyakov alexander-smolyakov requested a review from a team as a code owner November 29, 2023 10:12
Copy link
Member

@samruddhikhandale samruddhikhandale left a comment

Choose a reason for hiding this comment

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

This looks amazing, thanks for adding the new Feature option. Left some minor comments.

@@ -22,6 +22,11 @@
"default": "os-provided",
"description": "Select a Python version to install."
},
"additionalVersions": {
Copy link
Member

Choose a reason for hiding this comment

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

We intentionally did not publicize this option just because it was experimental. Also, it was meant only to be used by universal image.

However, if it works great in different test scenarios then it is worth publicizing 🎉

src/python/install.sh Outdated Show resolved Hide resolved
src/python/devcontainer-feature.json Show resolved Hide resolved
jdputschadi pushed a commit to analogdevicesinc/devcontainers-features that referenced this pull request Feb 7, 2024
samruddhikhandale pushed a commit that referenced this pull request Feb 9, 2024
* install gpg2.22 on centos 7 when installing python

* RHEL support, exisiting tests pass, new RHEL tests pass.

* add tests, cleanup install organization

* update testing to include RHEL tests

* update testing to include RHEL tests

* undo addition of installing additional pip modules

* fix errors installing os-provided Python on recent Debian systems and on Mariner systems

* adjust to properly use newly installed python (PYTHON_SRC) instead of assuming "python" will work

* When installing pipx, check if python is marked as externally
managed. If so, add "--break-system-packages" to the pip install
flags.

This does not really breack system packages due to the setting of
PYTHONUSERBASE during the install of pipx, but does get us past
checks for installing python packages into the system python install.

* merge from main

* update check for managed python install. pass all tests.

* add "packages" option from PR #768

* remove "packages" option

* Address PR feedback, passes all tests locally.

* fix install error on centos

---------

Co-authored-by: Jeff Putsch <[email protected]>
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.

2 participants