fix: Install Python 3.12 alongside system Python 3.9#137
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds a new ChangesPython 3.12 installation support
Suggested reviewers
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
dgchinner
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/tests_include_vars_from_parent.yml (1)
47-64: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse the mandated test wrapper instead of direct
import_role.This test still invokes a role directly via
import_role. Switch to the central wrapper task (tasks/run_role_with_clear_facts.yml) and pass these vars there, includinghpc_install_python312.As per path instructions: "CRITICAL: ... NEVER use
ansible.builtin.import_roledirectly ... ALWAYS use ...ansible.builtin.include_tasks: tasks/run_role_with_clear_facts.yml".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/tests_include_vars_from_parent.yml` around lines 47 - 64, The test task named "Import role" is using direct import_role instead of the mandated centralized wrapper. Replace the entire import_role task with an include_tasks task that calls tasks/run_role_with_clear_facts.yml instead, passing all the same vars (roletoinclude, hpc_manage_storage, hpc_install_nvidia_driver, hpc_install_cuda_driver, hpc_install_cuda_toolkit, hpc_install_hpc_nvidia_nccl, hpc_install_nvidia_fabric_manager, hpc_install_nvidia_dcgm, hpc_install_rdma, hpc_install_system_openmpi, hpc_build_mpi_w_nvidia_gpu_support, hpc_install_mvapich, hpc_install_mpifileutils, and hpc_install_python312) through the wrapper task instead of directly to import_role.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 64-76: The documentation for the hpc_install_python312 variable
contains two issues that need to be fixed. First, correct the version mismatch
in the explanation: the text currently states that Azure Slurm cluster
infrastructure requires Python 3.11, but the variable is for installing Python
3.12, so update this statement to correctly reflect that Python 3.12 is
required. Second, add a concrete usage example after the variable description
that demonstrates how to enable or configure hpc_install_python312 in a playbook
or configuration file, such as showing how to set the variable to true/false and
what the resulting behavior would be.
In `@tasks/main.yml`:
- Around line 82-94: The new task "Install Python 3.12 alongside system Python
3.9" that conditionally installs python3.12, python3.12-pip, and
python3.12-devel packages lacks test coverage for its positive execution path.
The existing test only sets hpc_install_python312 to false, so the task is never
actually exercised. Add at least one test case that sets hpc_install_python312
to true and validates that the package installation task executes successfully
and installs the expected Python 3.12 packages.
- Line 84: The package task at line 84 is using a short module name instead of
the fully qualified collection name. Replace the `package:` module declaration
with `ansible.builtin.package:` to comply with the role's task policy that
requires all tasks to use FQCN (Fully Qualified Collection Name) instead of
short module names like yum, dnf, or apt.
---
Outside diff comments:
In `@tests/tests_include_vars_from_parent.yml`:
- Around line 47-64: The test task named "Import role" is using direct
import_role instead of the mandated centralized wrapper. Replace the entire
import_role task with an include_tasks task that calls
tasks/run_role_with_clear_facts.yml instead, passing all the same vars
(roletoinclude, hpc_manage_storage, hpc_install_nvidia_driver,
hpc_install_cuda_driver, hpc_install_cuda_toolkit, hpc_install_hpc_nvidia_nccl,
hpc_install_nvidia_fabric_manager, hpc_install_nvidia_dcgm, hpc_install_rdma,
hpc_install_system_openmpi, hpc_build_mpi_w_nvidia_gpu_support,
hpc_install_mvapich, hpc_install_mpifileutils, and hpc_install_python312)
through the wrapper task instead of directly to import_role.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: ca4cc165-e201-4cd9-9c75-018be33305cd
📒 Files selected for processing (6)
README.mddefaults/main.ymltasks/main.ymltests/tests_default.ymltests/tests_include_vars_from_parent.ymltests/tests_skip_toolkit.yml
richm
reviewed
Jun 24, 2026
dgchinner
force-pushed
the
python-3.12-support
branch
from
d8ecdf3 to
97bcecf
Compare
richm
approved these changes
Jun 25, 2026
spetrosi
approved these changes
Jun 26, 2026
The Slurm cluster infrastructure that uses images built by this role requires Python 3.12. RHEL 9.6 only provides Python 3.9 as the default, so this change installs Python 3.12 from the AppStream repository alongside the system default. The system default Python is left at 3.9 to avoid breaking OS tools (firewall-cmd, dnf, semanage, etc.) that depend on Python 3.9 modules not available for 3.12 (e.g. PyGObject). Python 3.12 is available as /usr/bin/python3.12. Changes: - Add hpc_install_python312 variable (default: true) to defaults/main.yml - Add task to install python3.12, python3.12-pip, python3.12-devel - Add README.md documentation for the new variable - Disable python3.12 installation in CI test playbooks Created-by-AI: Claude Opus 4.6 (1M context) Prompt: new modification: the slurm cluster infrastructure that uses the build images requires support for python 3.12. RHEL 9.6 only provides python-3.9, so we need to install python-3.12 from the AppStream repository and set that to the default python version for the system. Refinements: - Removed alternatives configuration that set python3.12 as system default, as it breaks OS tools like firewall-cmd that depend on python3.9-only modules (PyGObject/gi) - Python 3.12 is installed alongside system python, not as a replacement Signed-off-by: Dave Chinner <dchinner@redhat.com>
dgchinner
force-pushed
the
python-3.12-support
branch
from
97bcecf to
e144383
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Slurm cluster infrastructure that uses images built by this role requires Python 3.11. RHEL 9.6 only provides Python 3.9 as the default, so this change installs Python 3.12 from the AppStream repository alongside the system default. The system default Python is left at 3.9 to avoid breaking OS tools (firewall-cmd, dnf, semanage, etc.) that depend on Python 3.9 modules not available for 3.12 (e.g. PyGObject). Python 3.12 is available as /usr/bin/python3.12.
Changes:
Created-by-AI: Claude Opus 4.6 (1M context)
Prompt: new modification: the slurm cluster infrastructure that uses the build images requires support for python 3.12. RHEL 9.6 only provides python-3.9, so we need to install python-3.12 from the AppStream repository and set that to the default python version for the system.
Refinements:
Issue Tracker Tickets (Jira or BZ if any): https://redhat.atlassian.net/browse/RHELHPC-206
Summary by CodeRabbit
New Features
Documentation
hpc_install_python312variable, including what it installs and its default behavior.Tests