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

edx-ora2 | Replace pkg_resources to importlib_resources | Drop support for python 3.8 #2238

Merged
merged 10 commits into from
Nov 12, 2024

Conversation

ttqureshi
Copy link
Contributor

@ttqureshi ttqureshi commented Sep 30, 2024

Overview

This PR:


Tickets:

Screenshots

image image

@ttqureshi ttqureshi requested a review from a team as a code owner September 30, 2024 07:04
- Drop support for python 3.8 and add support for python 3.11 and 3.12
requirements/common_constraints.txt Outdated Show resolved Hide resolved
.github/workflows/pypi-publish.yml Outdated Show resolved Hide resolved
@ttqureshi ttqureshi force-pushed the ttqureshi/depr_pkg_resources branch from a3a6b0c to ad7695b Compare October 1, 2024 09:23
@ttqureshi ttqureshi requested a review from farhan October 2, 2024 09:12
@pomegranited
Copy link
Contributor

Hi @ttqureshi -- I'm working my way through the PR backlog for this repo, apologies for letting this one go stale!
Could you resolve the conflicts so I can review & merge?

@ttqureshi
Copy link
Contributor Author

Hi @ttqureshi -- I'm working my way through the PR backlog for this repo, apologies for letting this one go stale! Could you resolve the conflicts so I can review & merge?

sure

@ttqureshi
Copy link
Contributor Author

conflicts resolved

Copy link

codecov bot commented Oct 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.29%. Comparing base (42e754e) to head (8ea48d4).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2238      +/-   ##
==========================================
+ Coverage   95.23%   95.29%   +0.05%     
==========================================
  Files         195      195              
  Lines       21606    21624      +18     
  Branches     1502     1502              
==========================================
+ Hits        20576    20606      +30     
+ Misses        784      771      -13     
- Partials      246      247       +1     
Flag Coverage Δ
unittests 95.29% <100.00%> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@pomegranited pomegranited left a comment

Choose a reason for hiding this comment

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

👍 Thank you for this change @ttqureshi -- it works perfectly.

One nit re the version bump, and could you update the tests to fix coverage?

  • I tested this in my tutor dev stack using the Demo course in Studio, LMS, and Preview
  • I read through the code
  • I checked for accessibility issues by using my keyboard to navigate
  • Includes documentation N/A
  • User-facing strings are extracted for translation N/A

openassessment/__init__.py Outdated Show resolved Hide resolved
@ttqureshi ttqureshi force-pushed the ttqureshi/depr_pkg_resources branch from 0f7a34d to 3593c0b Compare November 1, 2024 07:55
@pomegranited
Copy link
Contributor

Hi @ttqureshi -- I'm not sure why I have to keep allowing the CI to run here, but I bet it's frustrating to have to wait for me to know when you've fixed all the errors :(

You can run the tests locally using tox, e.g.

python3.11 -m venv venv311
source venv311/bin/activate
pip install tox
TOXENV=quality tox
TOXENV=django42 tox

If you want to run the 3.12 tests, use python 3.12 to create your virtualenv.

@ttqureshi
Copy link
Contributor Author

Thanks @pomegranited for the heads-up! I’ll run the tests locally with tox to catch any errors before pushing.
Appreciate the tips.

@ttqureshi ttqureshi force-pushed the ttqureshi/depr_pkg_resources branch from 098c1a3 to 7bbf574 Compare November 6, 2024 08:41
@ttqureshi ttqureshi requested a review from farhan November 6, 2024 08:43
@pomegranited
Copy link
Contributor

@ttqureshi we're so close now!

@pomegranited
Copy link
Contributor

So close @ttqureshi ! Just need a little more code coverage -- the easiest way to spot the missing lines is to look for the inline codecov warnings like these.

@ttqureshi
Copy link
Contributor Author

So close @ttqureshi ! Just need a little more code coverage -- the easiest way to spot the missing lines is to look for the inline codecov warnings like these.

alright 👍

@ttqureshi
Copy link
Contributor Author

@pomegranited I hope it's done now :)

@pomegranited
Copy link
Contributor

@ttqureshi Tests are all green now! Thank you so much for persisting with this change. Merging now :)

@pomegranited pomegranited merged commit 979d3f2 into openedx:master Nov 12, 2024
11 checks passed
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.

Drop Python 3.8 & Add Support for Python 3.12 Move on from deprecated pkg_resources api
3 participants