Skip to content

feat: add django 5.2 support #412

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Apr 11, 2025
Merged

feat: add django 5.2 support #412

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ jobs:
python-version:
- '3.11'
- '3.12'
toxenv: [django42-celery53-drflatest,
quality, docs, django42]
toxenv: [django42-celery53-drflatest, django52-celery54-drflatest,
quality, docs]

steps:
- uses: actions/checkout@v4
Expand All @@ -37,7 +37,7 @@ jobs:
run: tox

- name: Run coverage
if: matrix.python-version == '3.11' && matrix.toxenv == 'django42-celery53-drflatest'
if: matrix.python-version == '3.11' && matrix.toxenv == 'django52-celery54-drflatest'
uses: codecov/codecov-action@v5
with:
token: ${{ secrets.CODECOV_TOKEN }}
Expand Down
7 changes: 7 additions & 0 deletions CHANGELOG.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,13 @@ Change Log
Unreleased
~~~~~~~~~~

[3.4.0] - 2025-04-05
~~~~~~~~~~~~~~~~~~~~

Added
+++++++
* Added `Django 5.2` support

[3.3.0] - 2025-02-13
~~~~~~~~~~~~~~~~~~~~

Expand Down
6 changes: 3 additions & 3 deletions schema/urls.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from drf_yasg.views import get_schema_view

from django.contrib import admin
from django.urls import include, re_path
from django.urls import include, path, re_path

from rest_framework import permissions

Expand All @@ -24,7 +24,7 @@

# The Swagger/Open API JSON file can be found at http://localhost:8000/?format=openapi
urlpatterns = [
re_path(r'^admin/', include(admin.site.urls)),
path('admin/', include(admin.site.urls)),
re_path(r'^swagger(?P<format>\.json|\.yaml)$', SCHEMA.without_ui(cache_timeout=0), name='schema-json'),
re_path(r'^swagger/$', SCHEMA.with_ui('swagger', cache_timeout=0), name='schema-swagger-ui'),
path('swagger/', SCHEMA.with_ui('swagger', cache_timeout=0), name='schema-swagger-ui'),
] + base_patterns
1 change: 1 addition & 0 deletions setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ def is_requirement(line):
'Development Status :: 4 - Beta',
'Framework :: Django',
'Framework :: Django :: 4.2',
'Framework :: Django :: 5.2',
'Intended Audience :: Developers',
'License :: OSI Approved :: Apache Software License',
'Natural Language :: English',
Expand Down
6 changes: 6 additions & 0 deletions test_settings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,9 @@ def root(*args):
]

USE_TZ = True

STORAGES = {
"default": {
"BACKEND": "django.core.files.storage.FileSystemStorage",
},
}
3 changes: 2 additions & 1 deletion tox.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
[tox]
envlist =
py{311,312}-django{42}-celery{53}-drf{313,latest}
py{311,312}-django{42, 52}-celery{53, 54}-drf{313,latest}
quality
docs
[testenv]
deps =
django42: Django>=4.2,<4.3
django52: Django>=5.2,<5.3
drflatest: djangorestframework
-r{toxinidir}/requirements/test.txt
commands =
Expand Down
2 changes: 1 addition & 1 deletion user_tasks/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

from django.dispatch import Signal

__version__ = '3.3.0'
__version__ = '3.4.0'


# This signal is emitted when a user task reaches any final state:
Expand Down
11 changes: 9 additions & 2 deletions user_tasks/conf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,18 @@
from datetime import timedelta

from django.conf import settings as django_settings
from django.core.files.storage import get_storage_class
from django.core.files.storage import storages

from user_tasks import filters


def get_storage(import_path=None):
"""
Get the default storage backend or for the given import path.
"""
return storages["default"] if import_path is None else storages[import_path]
Copy link
Contributor

Choose a reason for hiding this comment

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

@UsamaSadiq you might need to run this on sandbox before merging in platform.

Copy link
Contributor

@awais786 awais786 May 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@UsamaSadiq

The current implementation might not work in production because there’s no STORAGES dictionary defined in the edx-platform production.py settings file and with default storage it will bring django default settings.
e.g

{'_backends': {'default': {'BACKEND': 'django.core.files.storage.FileSystemStorage'}, 'staticfiles': {'BACKEND': 'django.contrib.staticfiles.storage.StaticFilesStorage'}}, '_storages': {}, 'backends': {'default': {'BACKEND': 'django.core.files.storage.FileSystemStorage'}, 'staticfiles': {'BACKEND': 'django.contrib.staticfiles.storage.StaticFilesStorage'}}}

USER_TASKS_ARTIFACT_STORAGE uses the S3Boto3 storage class, as seen here:

https://github.com/openedx/edx-platform/blob/489dc774bb8f15881611ba398810fb6055c1b8f4/cms/envs/production.py#L276

With the current changes, the import path would resolve to cms.djangoapps.contentstore.storage.ImportExportS3Storage.

Attempting to use storages[import_path] will raise an InvalidStorageError because the STORAGES setting does not include this path as a registered alias.

To fix this and ensure it works in production, you should dynamically import the class using:

from django.utils.module_loading import import_string
import_string(import_path)

This approach does not rely on the STORAGES dictionary and is compatible with the current production configuration.

I would suggest you to add some tests in this repo. like Irtaza added in edx-val.

cc @irtazaakram

Copy link
Member Author

Choose a reason for hiding this comment

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

Going through the suggestion solution and creating a fix PR for this issue



class LazySettings():
"""
The behavior of ``django-user-tasks`` can be customized via the following Django settings.
Expand Down Expand Up @@ -37,7 +44,7 @@ def USER_TASKS_ARTIFACT_STORAGE(self): # pylint: disable=invalid-name
backend class.
"""
import_path = getattr(django_settings, 'USER_TASKS_ARTIFACT_STORAGE', None)
return get_storage_class(import_path)()
return get_storage(import_path)

@property
def USER_TASKS_MAX_AGE(self): # pylint: disable=invalid-name
Expand Down