-
Notifications
You must be signed in to change notification settings - Fork 6
fix: install vendored python 3.11 on ubuntu focal for analytics api #278
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR modifies the analytics API role to install Python 3.11 from vendored packages on Ubuntu Focal instead of using the standard repository installation. This is necessary because Ubuntu Focal (20.04) is out of support and Python 3.11 is not available in its default repositories.
Key changes:
- Switches from system Python 3.11 installation to vendored package installation controlled by a feature flag
- Adds tasks to download and install Python 3.11 packages from a vendored GitHub repository
- Disables the default Python 3.11 installation in the edx_django_service dependency
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| playbooks/roles/analytics_api/tasks/main.yml | Adds new tasks to conditionally install either standard or vendored Python 3.11 packages |
| playbooks/roles/analytics_api/meta/main.yml | Disables Python 3.11 installation in the edx_django_service role dependency |
| playbooks/roles/analytics_api/defaults/main.yml | Defines configuration variables for vendored Python 3.11 package installation including URLs, build versions, and dependencies |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| # | ||
|
|
||
| - name: install python3.11 | ||
| when: not use_vendored_py311 |
Copilot
AI
Oct 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition logic is inverted. When use_vendored_py311 is true, the standard Python 3.11 installation task should be skipped, but this condition will execute the standard installation when vendored packages are being used. Change to when: not use_vendored_py311 | default(false) or verify the intended logic.
| when: not use_vendored_py311 | |
| when: not use_vendored_py311 | default(false) |
| # Build string that's present in the deb file names. (Just used to make the | ||
| # names below easier to read.) | ||
| vendored_py311_build: "3.11.13-15-g8adac492d4-1+focal1" | ||
| # These must be kept topologically sorted, dependant packages last, as they |
Copilot
AI
Oct 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Corrected spelling of 'dependant' to 'dependent'.
| # These must be kept topologically sorted, dependant packages last, as they | |
| # These must be kept topologically sorted, dependent packages last, as they |
|
|
||
| - name: "Install vendored Python 3.11 packages" | ||
| when: use_vendored_py311 | ||
| command: dpkg -i "/tmp/vendored_python_{{ vendored_py311_build }}/{{ item }}" |
Copilot
AI
Oct 23, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using command module with dpkg -i will always report as 'changed' even if packages are already installed. Consider using the apt module with deb parameter or add creates parameter to make the task idempotent and prevent unnecessary reinstallations on subsequent runs.
| command: dpkg -i "/tmp/vendored_python_{{ vendored_py311_build }}/{{ item }}" | |
| apt: | |
| deb: "/tmp/vendored_python_{{ vendored_py311_build }}/{{ item }}" |
7bf1a02 to
6317524
Compare
5ab33b5 to
6f068ff
Compare
|
@timmc-edx Please review now. I have removed the changes from the analytics_api role and moved them inside |
6f068ff to
d96153a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
|
@timmc-edx Please review the new changes. |
timmc-edx
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that should do it!
d96153a to
844cb1a
Compare
|
@timmc-edx I am trying to build the Python3.12 for focal using the below steps mentioned in your commit but getting errors. Do I need to run the whole process on an ubuntu machine? I only have a Macbook. Could you please guide me how to build Python3.12 for focal? I started with below steps. First error Then I tried I tried to create and add gpg keys but it didn't work so I commented out |
|
Hmm! Interesting. I'll try building 3.12 today to see if it works on my machine. |
|
Yeah, works here, but I've used GPG on this machine and so it's seeing the .gnupg here. I wonder why it even checks, though... I don't see anything that's signing packages, and it would prompt for a passphrase if it were doing so. I was able to build after commenting out a few GPG-related lines, but I don't know if it's actually safe to do so. I've filed an issue to ask about that: deadsnakes/issues#331 In any case, try keeping those lines but first run |
Thank you so much for trying. Are you doing all of this on a MacBook? I will try this tomorrow. |
|
No, I have a Linux laptop -- Ubuntu Noble. So I can only build the amd64 images. I went ahead and uploaded an amd64 Python 3.12 build here, if you could review: edx/vendored#1 |
Description: Changes in this PR have been copied from #269, #270 and #272
Make sure that the following steps are done before merging: