Add Django 5.2 support and compatibility fixes by mfarhan943 · Pull Request #4646 · openedx/course-discovery

mfarhan943 · 2025-06-20T10:05:41Z

Description

Upgraded codebase to support Django 5.2
Resolved attribute errors on StdImageFieldFile
Added setting ITERATOR_CHUNK_SIZE for Django 5.2 compatibility

Testing Instructions

Refactor code to support django 5.2
git ls-files -z -- '*.py' | xargs -0r django-upgrade --target-version 5.2
Using django 4.2 run the following command in container shell
python manage.py shell

Expected result:
Django shell will load without any error.

DEPR: #4668

mfarhan943 · 2025-06-20T10:12:41Z

course_discovery/settings/base.py


 USE_I18N = True

-USE_L10N = True


We currently have USE_L10N = True in this repository. Since this setting is enabled by default in Django 4.2 and removed entirely in Django 5.2, it is safe to remove without causing any issues in production.

mfarhan943 · 2025-06-20T11:22:39Z

course_discovery/apps/course_metadata/search_indexes/documents/person.py

        """

        model = Person
+        queryset_pagination = settings.ELASTICSEARCH_DSL_QUERYSET_PAGINATION


queryset_pagination was in Meta, which is ignored by django-elasticsearch-dsl. Moved it to the Django class as per the documentation.

mfarhan943 · 2025-06-20T11:23:10Z

course_discovery/apps/course_metadata/signals.py

        course__degree_course_curricula=instance,
        external_key__isnull=False
-    ).iterator()
+    ).iterator(chunk_size=settings.ITERATOR_CHUNK_SIZE)


In Django 5.0, QuerySet.iterator() requires an explicit chunk_size, especially when using prefetches. I set it to 2000 to match the previous default value used in earlier versions. Release Notes 5.0

mfarhan943 · 2025-06-20T11:23:55Z

course_discovery/apps/course_metadata/tests/test_admin.py

        program_type = factories.ProgramTypeFactory(applicable_seat_types=[verified_seat_type])
-        program_filter = ProgramEligibilityFilter(None, {self.parameter_name: 1}, None, None)
+        value = [1] if DJANGO_VERSION >= (5, 2) else 1
+        program_filter = ProgramEligibilityFilter(None, {self.parameter_name: value}, None, None)


Django 5.0+ returns a tuple from SimpleListFilter.value(), even for single values.
Release note
This change in django 5.0 is for a bug fix discussed in the forums.

awais786 · 2025-07-25T02:50:15Z

course_discovery/settings/production.py

    # than pumping them into the local vars.
    dict_updates = {key: config_from_yaml.pop(key, None) for key in DICT_UPDATE_KEYS}

+    # Extract legacy storage backend settings from yaml config to avoid conflicts.


Need to verify this on sandbox or stage.

awais786 · 2025-08-25T18:36:25Z

@feanil Please review. This is first of django52 compatibility.

feanil

Changes generally make sense but I'm not sure why the tests aren't running.

feanil · 2025-08-29T14:15:33Z

Maybe do a rebase here since there is also a conflict that needs to be resolved?

openedx-webhooks · 2025-08-29T14:15:38Z

Thanks for the pull request, @mfarhan943!

This repository is currently maintained by @openedx/2u-phoenix.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
- This process (including the steps you'll need to take) is documented here.
If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

Dependencies

This PR must be merged before / after / at the same time as ...
Blockers

This PR is waiting for OEP-1234 to be accepted.
Timeline information

This PR must be merged by XX date because ...
Partner information

This is for a course on edx.org.
Supporting documentation
Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

The size and impact of the changes that it introduces
The need for product review
Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

feanil · 2025-09-05T14:40:05Z

.github/workflows/ci.yml

Looks like the tests are failing in part because you're trying to upload coverage related artifacts with the same name twice. I think the correct thing to do is only publish coverage data from the latest version so, I think update the conditional for the coverage related tasks to only run on django-version == 'django52'.

awais786 · 2025-09-10T09:41:40Z

@feanil Please review.

feanil · 2025-09-15T15:16:50Z

@awais786 does this need the same kind of DEPR similar to openedx/openedx-platform#37333 for this repo for operators?

feanil · 2025-09-15T15:18:10Z

Makefile

 	# Let tox control the Django version for tests
-	grep -e "^django==" requirements/local.txt > requirements/django.txt
+	sed -i.tmp '/^[dD]jango==/d' requirements/test.txt
+	sed -i.tmp '/^djangorestframework==/d' requirements/test.txt


Is this introducing a bug? It looks like the djangorestframework requirement is removed but not added back in via Tox to any other version. So we're no longer explicitly setting the version and likely getting whatever version is coming from other base requirements. I think we don't want that.

awais786 · 2025-09-16T10:49:20Z

openedx/openedx-platform#37333

Yes, let me add for this and credentials also.

feanil · 2025-09-17T16:14:10Z

Changes generally look good, please link the relevant DEPR here once we have it. Then I think this can probably be approved and merged.

mfarhan943 · 2025-09-18T18:34:59Z

Changes generally look good, please link the relevant DEPR here once we have it. Then I think this can probably be approved and merged.

Done

- Upgraded codebase to support Django 5.2 - Resolved attribute errors on StdImageFieldFile - Added setting ITERATOR_CHUNK_SIZE for Django 5.2 compatibility

…les.

feanil · 2025-09-18T19:47:40Z

@awais786 or @mfarhan943 can you make the followup PR that makes django 5.2 the default once this lands?

awais786 · 2025-09-19T06:07:57Z

@feanil we will fix STORAGES stuff PR first like we did in credentials. So we will make it today. After that we will make default PR for django52

mfarhan943 commented Jun 20, 2025

View reviewed changes

mfarhan943 force-pushed the farhan-upgrade-django52 branch 4 times, most recently from f93df2d to a943b4c Compare June 27, 2025 15:16

awais786 requested a review from feanil July 23, 2025 00:36

mfarhan943 force-pushed the farhan-upgrade-django52 branch from 875a44b to 16d23d8 Compare July 23, 2025 07:27

UsamaSadiq mentioned this pull request Jul 23, 2025

course-discovery: Add django52 support. openedx/public-engineering#371

Open

awais786 reviewed Jul 25, 2025

View reviewed changes

awais786 added the create-sandbox open-craft-grove should create a sandbox environment from this PR label Aug 6, 2025

mfarhan943 force-pushed the farhan-upgrade-django52 branch 3 times, most recently from 67fc0ae to a10c65a Compare August 19, 2025 14:23

feanil closed this Aug 28, 2025

feanil reopened this Aug 28, 2025

feanil reviewed Aug 29, 2025

View reviewed changes

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Aug 29, 2025

openedx-webhooks added this to Contributions Aug 29, 2025

github-project-automation bot moved this to Needs Triage in Contributions Aug 29, 2025

mfarhan943 force-pushed the farhan-upgrade-django52 branch from 96067c4 to 41cf4c4 Compare August 31, 2025 18:28

awais786 mentioned this pull request Sep 4, 2025

Django 5.2 Upgrade openedx/public-engineering#339

Open

mfarhan943 force-pushed the farhan-upgrade-django52 branch from 41cf4c4 to ebafa9c Compare September 4, 2025 11:41

feanil reviewed Sep 5, 2025

View reviewed changes

mfarhan943 force-pushed the farhan-upgrade-django52 branch from ebafa9c to f613861 Compare September 5, 2025 19:25

mphilbrick211 requested a review from feanil September 10, 2025 20:39

mphilbrick211 moved this from Needs Triage to Ready for Review in Contributions Sep 10, 2025

feanil reviewed Sep 15, 2025

View reviewed changes

mfarhan943 force-pushed the farhan-upgrade-django52 branch from 565ab8c to 0f3be0e Compare September 16, 2025 18:34

feanil approved these changes Sep 18, 2025

View reviewed changes

awais786 and others added 5 commits September 18, 2025 15:46

feat!: add Django 5.2 support and compatibility fixes

a658ff2

- Upgraded codebase to support Django 5.2 - Resolved attribute errors on StdImageFieldFile - Added setting ITERATOR_CHUNK_SIZE for Django 5.2 compatibility

fix: install pycparser to resolve cairocffi build error

06768d1

refactor: remove dict-based overrides, use direct setting variables

77f6115

fix: refactor: remove dict-based overrides, use direct setting variab…

dde7472

…les.

fix(test-requirements): keep djangorestframework pinned

a14d50e

feanil force-pushed the farhan-upgrade-django52 branch from 0f3be0e to a14d50e Compare September 18, 2025 19:46

feanil enabled auto-merge (rebase) September 18, 2025 19:47

feanil merged commit 0345eb4 into openedx:master Sep 18, 2025
18 checks passed

github-project-automation bot moved this from Ready for Review to Done in Contributions Sep 18, 2025

Comments

Conversation

mfarhan943 commented Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing Instructions

DEPR: #4668

Uh oh!

mfarhan943 Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mfarhan943 Jun 20, 2025

Choose a reason for hiding this comment

Uh oh!

mfarhan943 Jun 20, 2025

Choose a reason for hiding this comment

Uh oh!

mfarhan943 Jun 20, 2025

Choose a reason for hiding this comment

Uh oh!

awais786 Jul 25, 2025

Choose a reason for hiding this comment

Uh oh!

awais786 commented Aug 25, 2025

Uh oh!

feanil left a comment

Choose a reason for hiding this comment

Uh oh!

feanil commented Aug 29, 2025

Uh oh!

openedx-webhooks commented Aug 29, 2025

Uh oh!

feanil Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

awais786 commented Sep 10, 2025

Uh oh!

feanil commented Sep 15, 2025

Uh oh!

feanil Sep 15, 2025

Choose a reason for hiding this comment

Uh oh!

awais786 commented Sep 16, 2025

Uh oh!

feanil commented Sep 17, 2025

Uh oh!

mfarhan943 commented Sep 18, 2025

Uh oh!

feanil commented Sep 18, 2025

Uh oh!

Uh oh!

awais786 commented Sep 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mfarhan943 commented Jun 20, 2025 •

edited

Loading

mfarhan943 Jun 20, 2025 •

edited

Loading

awais786 commented Sep 19, 2025 •

edited

Loading