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

Upgrade django-stubs again, unpin mypy, and add more annotations #36072

Merged
merged 3 commits into from
Jan 9, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 3 additions & 2 deletions openedx/core/djangoapps/content_libraries/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@
from openedx.core.lib.api.view_utils import view_auth_classes
from openedx.core.djangoapps.safe_sessions.middleware import mark_user_change_as_expected
from openedx.core.djangoapps.xblock import api as xblock_api
from openedx.core.types.http import RestRequest

from .models import ContentLibrary, LtiGradedResource, LtiProfile

Expand Down Expand Up @@ -667,7 +668,7 @@ class LibraryBlockCollectionsView(APIView):
View to set collections for a component.
"""
@convert_exceptions
def patch(self, request, usage_key_str) -> Response:
def patch(self, request: RestRequest, usage_key_str) -> Response:
"""
Sets Collections for a Component.

Expand All @@ -688,7 +689,7 @@ def patch(self, request, usage_key_str) -> Response:
library_key=key.lib_key,
component=component,
collection_keys=collection_keys,
created_by=self.request.user.id,
created_by=request.user.id,
content_library=content_library,
)

Expand Down
17 changes: 9 additions & 8 deletions openedx/core/djangoapps/content_libraries/views_collections.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
ContentLibraryCollectionComponentsUpdateSerializer,
ContentLibraryCollectionUpdateSerializer,
)
from openedx.core.types.http import RestRequest


class LibraryCollectionsView(ModelViewSet):
Expand Down Expand Up @@ -89,23 +90,23 @@ def get_object(self) -> Collection:
return collection

@convert_exceptions
def retrieve(self, request, *args, **kwargs) -> Response:
def retrieve(self, request: RestRequest, *args, **kwargs) -> Response:
"""
Retrieve the Content Library Collection
"""
# View declared so we can wrap it in @convert_exceptions
return super().retrieve(request, *args, **kwargs)

@convert_exceptions
def list(self, request, *args, **kwargs) -> Response:
def list(self, request: RestRequest, *args, **kwargs) -> Response:
"""
List Collections that belong to Content Library
"""
# View declared so we can wrap it in @convert_exceptions
return super().list(request, *args, **kwargs)

@convert_exceptions
def create(self, request, *args, **kwargs) -> Response:
def create(self, request: RestRequest, *args, **kwargs) -> Response:
"""
Create a Collection that belongs to a Content Library
"""
Expand Down Expand Up @@ -139,7 +140,7 @@ def create(self, request, *args, **kwargs) -> Response:
return Response(serializer.data)

@convert_exceptions
def partial_update(self, request, *args, **kwargs) -> Response:
def partial_update(self, request: RestRequest, *args, **kwargs) -> Response:
"""
Update a Collection that belongs to a Content Library
"""
Expand All @@ -161,7 +162,7 @@ def partial_update(self, request, *args, **kwargs) -> Response:
return Response(serializer.data)

@convert_exceptions
def destroy(self, request, *args, **kwargs) -> Response:
def destroy(self, request: RestRequest, *args, **kwargs) -> Response:
"""
Soft-deletes a Collection that belongs to a Content Library
"""
Expand All @@ -176,7 +177,7 @@ def destroy(self, request, *args, **kwargs) -> Response:

@convert_exceptions
@action(detail=True, methods=['post'], url_path='restore', url_name='collection-restore')
def restore(self, request, *args, **kwargs) -> Response:
def restore(self, request: RestRequest, *args, **kwargs) -> Response:
"""
Restores a soft-deleted Collection that belongs to a Content Library
"""
Expand All @@ -191,7 +192,7 @@ def restore(self, request, *args, **kwargs) -> Response:

@convert_exceptions
@action(detail=True, methods=['delete', 'patch'], url_path='components', url_name='components-update')
def update_components(self, request, *args, **kwargs) -> Response:
def update_components(self, request: RestRequest, *args, **kwargs) -> Response:
"""
Adds (PATCH) or removes (DELETE) Components to/from a Collection.

Expand All @@ -209,7 +210,7 @@ def update_components(self, request, *args, **kwargs) -> Response:
content_library=content_library,
collection_key=collection_key,
usage_keys=usage_keys,
created_by=self.request.user.id,
created_by=request.user.id,
remove=(request.method == "DELETE"),
)

Expand Down
4 changes: 2 additions & 2 deletions openedx/core/djangoapps/content_staging/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def save_xblock_to_user_clipboard(block: XBlock, user_id: int, version_num: int
olx=block_data.olx_str,
display_name=block_metadata_utils.display_name_with_default(block),
suggested_url_name=usage_key.block_id,
tags=block_data.tags,
tags=block_data.tags or {},
version_num=(version_num or 0),
)
(clipboard, _created) = _UserClipboard.objects.update_or_create(user_id=user_id, defaults={
Expand Down Expand Up @@ -209,7 +209,7 @@ def _user_clipboard_model_to_data(clipboard: _UserClipboard) -> UserClipboardDat
status=content.status,
block_type=content.block_type,
display_name=content.display_name,
tags=content.tags,
tags=content.tags or {},
version_num=content.version_num,
),
source_usage_key=clipboard.source_usage_key,
Expand Down
4 changes: 3 additions & 1 deletion openedx/core/djangoapps/content_staging/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,9 @@ class Meta:
version_num = models.PositiveIntegerField(default=0)

# Tags applied to the original source block(s) will be copied to the new block(s) on paste.
tags = models.JSONField(null=True, help_text=_("Content tags applied to these blocks"))
tags: models.JSONField[dict | None, dict | None] = models.JSONField(
null=True, help_text=_("Content tags applied to these blocks")
)

@property
def olx_filename(self) -> str:
Expand Down
7 changes: 4 additions & 3 deletions openedx/core/djangoapps/content_tagging/rest_api/v1/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
from rest_framework import status
from rest_framework.decorators import action
from rest_framework.exceptions import PermissionDenied, ValidationError
from rest_framework.request import Request
from rest_framework.response import Response
from rest_framework.views import APIView
from openedx_events.content_authoring.data import ContentObjectData, ContentObjectChangedData
Expand All @@ -19,6 +18,8 @@
CONTENT_OBJECT_TAGS_CHANGED,
)

from openedx.core.types.http import RestRequest

from ...auth import has_view_object_tags_access
from ...api import (
create_taxonomy,
Expand Down Expand Up @@ -99,7 +100,7 @@ def perform_create(self, serializer):
serializer.instance = create_taxonomy(**serializer.validated_data, orgs=user_admin_orgs)

@action(detail=False, url_path="import", methods=["post"])
def create_import(self, request: Request, **kwargs) -> Response: # type: ignore
def create_import(self, request: RestRequest, **kwargs) -> Response: # type: ignore
"""
Creates a new taxonomy with the given orgs and imports the tags from the uploaded file.
"""
Expand Down Expand Up @@ -183,7 +184,7 @@ class ObjectTagExportView(APIView):
""""
View to export a CSV with all children and tags for a given course/context.
"""
def get(self, request: Request, **kwargs) -> StreamingHttpResponse:
def get(self, request: RestRequest, **kwargs) -> StreamingHttpResponse:
"""
Export a CSV with all children and tags for a given course/context.
"""
Expand Down
45 changes: 45 additions & 0 deletions openedx/core/types/http.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
"""
Typing utilities for the HTTP requests, responses, etc.

Includes utilties to work with both vanilla django as well as djangorestframework.
"""
from __future__ import annotations

import django.contrib.auth.models # pylint: disable=imported-auth-user
import django.http
import rest_framework.request

import openedx.core.types.user
from openedx.core.types.meta import type_annotation_only


@type_annotation_only
class HttpRequest(django.http.HttpRequest):
"""
A request which either has a concrete User (from django.contrib.auth) or is anonymous.
"""
user: openedx.core.types.User


@type_annotation_only
class AuthenticatedHttpRequest(HttpRequest):
"""
A request which is guaranteed to have a concrete User (from django.contrib.auth).
"""
user: django.contrib.auth.models.User


@type_annotation_only
class RestRequest(rest_framework.request.Request):
"""
Same as HttpRequest, but extended for rest_framework views.
"""
user: openedx.core.types.User


@type_annotation_only
class AuthenticatedRestRequest(RestRequest):
"""
Same as AuthenticatedHttpRequest, but extended for rest_framework views.
"""
user: django.contrib.auth.models.User
37 changes: 37 additions & 0 deletions openedx/core/types/meta.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
"""
Typing utilities for use on other typing utilities.
"""
from __future__ import annotations

import typing as t


def type_annotation_only(cls: type) -> type:
"""
Decorates class which should only be used in type annotations.

This is useful when you want to enhance an existing 3rd-party concrete class with
type annotations for its members, but don't want the enhanced class to ever actually
be instantiated. For examples, see openedx.core.types.http.
"""
if t.TYPE_CHECKING:
return cls
return _forbid_init(cls)


def _forbid_init(forbidden: type) -> type:
kdmccormick marked this conversation as resolved.
Show resolved Hide resolved
"""
Return a class which refuses to be instantiated.
"""
class _ForbidInit:
"""
The resulting class.
"""
def __init__(self, *args, **kwargs):
raise NotImplementedError(
f"Class {forbidden.__module__}:{forbidden.__name__} "
"cannot be instantiated. You may use it as a type annotation, but objects "
"can only be created from its concrete superclasses."
)

return _ForbidInit
6 changes: 4 additions & 2 deletions openedx/core/types/user.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
"""
Typing utilities for the User models.
"""
from typing import Union
from __future__ import annotations

import typing as t

import django.contrib.auth.models

User = Union[django.contrib.auth.models.User, django.contrib.auth.models.AnonymousUser]
User: t.TypeAlias = django.contrib.auth.models.User | django.contrib.auth.models.AnonymousUser
14 changes: 5 additions & 9 deletions requirements/constraints.txt
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,13 @@ django-webpack-loader==0.7.0
djangorestframework<3.15.0

# Date: 2024-07-19
# Generally speaking, the major version of django-stubs should match the major version of django.
# Specifically, we need to perpetually constrain django-stubs to a compatible version based on:
# Generally speaking, the major version of django-stubs must either match the major version
# of django, or exceed it by 1. So, we will need to perpetually constrain django-stubs and
# update it as we perform django upgrades. For more details, see:
# https://github.com/typeddjango/django-stubs?tab=readme-ov-file#version-compatibility
# including the note on "Partial Support".
# Issue: https://github.com/openedx/edx-platform/issues/35275
django-stubs<5
django-stubs<6

# Date: 2024-07-23
# django-storages==1.14.4 breaks course imports
Expand Down Expand Up @@ -112,12 +114,6 @@ markdown<3.4.0
# Issue for unpinning: https://github.com/openedx/edx-platform/issues/35270
moto<5.0

# Date: 2024-10-16
# MyPY 1.12.0 fails on all PRs with the following error:
# openedx/core/djangoapps/content_libraries/api.py:732: error: INTERNAL ERROR
# Issue for unpinning: https://github.com/openedx/edx-platform/issues/35667
mypy<1.12.0

# Date: 2024-07-16
# We need to upgrade the version of elasticsearch to atleast 7.15 before we can upgrade to Numpy 2.0.0
# Otherwise we see a failure while running the following command:
Expand Down
13 changes: 5 additions & 8 deletions requirements/edx/development.txt
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ asgiref==3.8.1
# django
# django-cors-headers
# django-countries
# django-stubs
asn1crypto==1.5.1
# via
# -r requirements/edx/doc.txt
Expand Down Expand Up @@ -577,7 +578,7 @@ django-storages==1.14.3
# -r requirements/edx/doc.txt
# -r requirements/edx/testing.txt
# edxval
django-stubs==4.2.7
django-stubs==5.1.1
# via
# -c requirements/edx/../constraints.txt
# -r requirements/edx/development.in
Expand Down Expand Up @@ -625,7 +626,7 @@ djangorestframework==3.14.0
# openedx-learning
# ora2
# super-csv
djangorestframework-stubs==3.14.5
djangorestframework-stubs==3.15.2
# via -r requirements/edx/development.in
djangorestframework-xml==2.0.0
# via
Expand Down Expand Up @@ -1292,10 +1293,8 @@ multidict==6.1.0
# -r requirements/edx/testing.txt
# aiohttp
# yarl
mypy==1.11.2
# via
# -c requirements/edx/../constraints.txt
# -r requirements/edx/development.in
mypy==1.14.1
# via -r requirements/edx/development.in
mypy-extensions==1.0.0
# via mypy
mysqlclient==2.2.6
Expand Down Expand Up @@ -2119,8 +2118,6 @@ tqdm==4.67.1
# -r requirements/edx/testing.txt
# nltk
# openai
types-pytz==2024.2.0.20241221
# via django-stubs
types-pyyaml==6.0.12.20241230
# via
# django-stubs
Expand Down
Loading