Skip to content
This repository has been archived by the owner on Nov 4, 2024. It is now read-only.

build: add python 3.11 and 3.12 ci checks #254

Conversation

Danyal-Faheem
Copy link
Contributor

@Danyal-Faheem Danyal-Faheem commented Apr 17, 2024

Changes

  • Added CI checks as part of the upgrade process to python 3.11 and python 3.12.
  • Also fixes one part of the downstream issue in tutor-ecommerce.
  • Updated docker-compose to docker compose.
  • Added a check on code coverage to only run for python 3.8 as we don't need to unnecessarily run it for every python verrsion.

⛔️ MAIN BRANCH WARNING! 2U EMPLOYEES must make branches against the 2u/main BRANCH

  • I have checked the branch to which I would like to merge.

⛔️ DEPRECATION WARNING

This repository is deprecated and in maintainence-only operation while we work on a replacement, please see this announcement for more information.

Although we have stopped integrating new contributions, we always appreciate security disclosures and patches sent to [email protected]

If you're merging to master (not 2u/main) branch...

Merge checklist:

  • Any new requirements are in the right place (do not manually modify the requirements/*.txt files)
    • base.in if needed in production
    • test.in for test requirements
    • make upgrade && make requirements have been run to regenerate requirements
  • Version bumped

Post merge:

  • Tag pushed and a new version released
    • Note: Assets will be added automatically. You just need to provide a tag (should match your version number) and title and description.
  • After versioned build finishes in GitHub CI, verify version has been pushed to PyPI
    • Each step in the release build has a condition flag that checks if the rest of the steps are done and if so will deploy to PyPi.
      (so basically once your build finishes, after maybe a minute you should see the new version in PyPi automatically (on refresh))
  • PR created in ecommerce to upgrade dependencies (including ecommerce-worker)
    • This must be done after the version is visible in PyPi as make upgrade in ecommerce will look for the latest version in PyPi.
    • Note: the ecommerce-worker constraint in ecommerce must also be bumped to the latest version in PyPi.
  • Deploy ecommerce
  • Deploy ecomworker

@Danyal-Faheem Danyal-Faheem requested a review from a team as a code owner April 17, 2024 11:44
@DawoudSheraz
Copy link

@feanil Hi. Can you run the workflows on this PR, please? Thanks

Copy link

@johanseto johanseto left a comment

Choose a reason for hiding this comment

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

Hi I run in a fork this PR and I have 2 comments:

  • The lint commit would fail because ci is not in the allowed words for commitlint. You can change it to test.
    image

  • I want to understand if the tests would run in python 3.11 and 3.12.
    Because In those tests the test_target shows 3.8
    2024-04-30_10-31

Also, I suspect in quality target tests are using the python3.8.
2024-04-30_10-47

I think the problem is that the quality env is in python3.12, but the tests are running in the edxops/ecomworker docker image and there is 3.8.

Also this line https://github.com/openedx/ecommerce-worker/blob/f852676a216b35ee845af66eb142ff657d72f98f/.github/workflows/ci.yml#L33
The python3-dev package seems installing with python3.8
image

@Danyal-Faheem Danyal-Faheem changed the title ci: add python 3.11 and 3.12 checks build: add python 3.11 and 3.12 ci checks May 3, 2024
@feanil feanil linked an issue May 9, 2024 that may be closed by this pull request
@feanil
Copy link
Contributor

feanil commented May 9, 2024

@Danyal-Faheem looks like you need to fix the commit messages so the conform to OEP-51 and rebase this on top of master and then it should be good to merge. Let me know when that's done and I can help land it.

@DawoudSheraz
Copy link

DawoudSheraz commented May 10, 2024

@Danyal-Faheem looks like you need to fix the commit messages so the conform to OEP-51 and rebase this on top of master and then it should be good to merge. Let me know when that's done and I can help land it.

Hey Feanil. It needs some changes in Dockerfile (just like Discovery and e-commerce repositories). It's on our radar, we will try to get it done as soon as possible. Once merged, we will backport this to redwood.master.

@Danyal-Faheem Danyal-Faheem force-pushed the danyalfaheem/add-python-3.11-ci-checks branch from b51300d to c823f22 Compare May 10, 2024 10:20
@Danyal-Faheem
Copy link
Contributor Author

Hi @feanil, I've made the required changes, can you trigger the workflows here and also review this PR for me? Thank you.

Copy link

@johanseto johanseto left a comment

Choose a reason for hiding this comment

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

After the last commit, tests are also running in python 3.11 and 3.12 =)
2024-05-24_18-05
2024-05-24_18-02
2024-05-24_18-00

tox would enforce python3.8 without this change
Now we install tox in that specific python and then run tox from it
@Danyal-Faheem Danyal-Faheem force-pushed the danyalfaheem/add-python-3.11-ci-checks branch from 6e486ff to 22b6df1 Compare May 27, 2024 11:00
@Danyal-Faheem
Copy link
Contributor Author

Hi @feanil, this PR is ready to be merged. Can you take a look and merge this for me? Thanks.

@feanil feanil merged commit 94c79db into openedx-unsupported:master Jun 4, 2024
15 checks passed
@feanil
Copy link
Contributor

feanil commented Jun 4, 2024

@Danyal-Faheem please tag me on the backport PR as well though I think the BTR release manager can merge that as well I just want to know when it lands.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ecommerce-worker] Test Python 3.11 and 3.12
4 participants