Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: expand mypy static type checking (#32591)

Browse Source 
* chore: typing + fixes for content_staging
* chore: typing + fixes for learning_sequences
* chore: typing + fixes for content_libraries
* chore: typing + fixes for new XBlock runtime
* feat: type hinting more code with mypy
This commit is contained in:
Braden MacDonald
2023-07-19 09:58:19 -07:00
committed by GitHub
parent 92b684004e
commit 57420ed613
16 changed files with 118 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
mypy.ini
View File

@@ -3,4 +3,16 @@ follow_imports = silent
ignore_missing_imports = True
allow_untyped_globals = True
exclude = tests
files = openedx/core/djangoapps/content/learning_sequences/,openedx/core/types
plugins =
mypy_django_plugin.main,
mypy_drf_plugin.main
files =
openedx/core/djangoapps/content/learning_sequences/,
openedx/core/djangoapps/content_staging,
openedx/core/djangoapps/content_libraries,
openedx/core/djangoapps/xblock,
openedx/core/types
[mypy.plugins.django-stubs]
# content_staging only works with CMS; others work with either, so we run mypy with CMS settings.
django_settings_module = "cms.envs.test"

2
openedx/core/djangoapps/content/learning_sequences/data.py
View File

@@ -213,7 +213,7 @@ class CourseOutlineData:
course_visibility: CourseVisibility = attr.ib(validator=attr.validators.in_(CourseVisibility))
# Entrance Exam ID
entrance_exam_id = attr.ib(type=str)
entrance_exam_id = attr.ib(type=Optional[str])
def __attrs_post_init__(self):
"""Post-init hook that validates and inits the `sequences` field."""

2
openedx/core/djangoapps/content/learning_sequences/migrations/0014_remove_user_partition_group_duplicates.py
View File

@@ -1,5 +1,5 @@
# Generated by Django 2.2.24 on 2021-07-07 18:34
# type: ignore
from django.db import migrations
from django.db.models import Count, Min

2
openedx/core/djangoapps/content/learning_sequences/views.py
View File

@@ -201,7 +201,7 @@ class CourseOutlineView(APIView):
# Just like in masquerading, set real_user so that the
# SafeSessions middleware can see that the user didn't
# change unexpectedly.
target_user.real_user = request.user
target_user.real_user = request.user # type: ignore
return target_user
_course_masquerade, user = setup_masquerade(request, course_key, has_staff_access)

8
openedx/core/djangoapps/content_libraries/api.py
View File

@@ -268,14 +268,6 @@ class LibraryBundleLink:
opaque_key = attr.ib(type=LearningContextKey, default=None)
class AccessLevel: # lint-amnesty, pylint: disable=function-redefined
""" Enum defining library access levels/permissions """
ADMIN_LEVEL = ContentLibraryPermission.ADMIN_LEVEL
AUTHOR_LEVEL = ContentLibraryPermission.AUTHOR_LEVEL
READ_LEVEL = ContentLibraryPermission.READ_LEVEL
NO_ACCESS = None
# General APIs
# ============

4
openedx/core/djangoapps/content_staging/admin.py
View File

@@ -30,12 +30,12 @@ class UserClipboardAdmin(admin.ModelAdmin):
search_fields = ('user__username', 'source_usage_key', 'content__display_name')
readonly_fields = ('source_context_key', 'get_source_context_title')
@admin.display(description='Content')
def content_link(self, obj):
""" Display the StagedContent object as a link """
url = reverse('admin:content_staging_stagedcontent_change', args=[obj.content.pk])
return format_html('<a href="{}">{}</a>', url, obj.content)
content_link.short_description = 'Content'
@admin.display(description='Source Context Title')
def get_source_context_title(self, obj):
return obj.get_source_context_title()
get_source_context_title.short_description = 'Source Context Title'

4
openedx/core/djangoapps/content_staging/api.py
View File

@@ -44,7 +44,7 @@ def get_user_clipboard(user_id: int, only_ready: bool = True) -> UserClipboardDa
)
def get_user_clipboard_json(user_id: int, request: HttpRequest = None):
def get_user_clipboard_json(user_id: int, request: HttpRequest | None = None):
"""
Get the detailed status of the user's clipboard, in exactly the same format
as returned from the
@@ -88,7 +88,7 @@ def get_staged_content_static_files(staged_content_id: int) -> list[StagedConten
sc = _StagedContent.objects.get(pk=staged_content_id)
def str_to_key(source_key_str: str):
if not str:
if not source_key_str:
return None
try:
return AssetKey.from_string(source_key_str)

1
openedx/core/djangoapps/content_staging/models.py
View File

@@ -1,6 +1,7 @@
"""
Models for content staging (and clipboard)
"""
from __future__ import annotations
import logging
from django.contrib.auth import get_user_model

2
openedx/core/djangoapps/content_staging/views.py
View File

@@ -189,7 +189,7 @@ class ClipboardEndpoint(APIView):
content: bytes | None = f.data
md5_hash = "" # Unknown
if content:
md5_hash = hashlib.md5(f.data).hexdigest()
md5_hash = hashlib.md5(content).hexdigest()
# This asset came from the XBlock's filesystem, e.g. a video block's transcript file
source_key = usage_key
# Check if the asset file exists. It can be absent if an XBlock contains an invalid link.

7
openedx/core/djangoapps/xblock/apps.py
View File

@@ -1,12 +1,11 @@
"""
Django app configuration for the XBlock Runtime django app
"""
from django.apps import AppConfig, apps
from django.conf import settings
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
from .data import StudentDataMode
class XBlockAppConfig(AppConfig):
@@ -53,7 +52,7 @@ class LmsXBlockAppConfig(XBlockAppConfig):
editing XBlock content in the LMS
"""
return dict(
student_data_mode='persisted',
student_data_mode=StudentDataMode.Persisted,
)
def get_site_root_url(self):
@@ -77,7 +76,7 @@ class StudioXBlockAppConfig(XBlockAppConfig):
editing XBlock content in Studio
"""
return dict(
student_data_mode='ephemeral',
student_data_mode=StudentDataMode.Ephemeral,
)
def get_site_root_url(self):

13
openedx/core/djangoapps/xblock/data.py Normal file
View File

@@ -0,0 +1,13 @@
"""
Data structures for the XBlock Django app's python APIs
"""
from enum import Enum
class StudentDataMode(Enum):
"""
Is student data (like which answer was selected) persisted in the DB or just stored temporarily in the session?
Generally, the LMS uses persistence and Studio uses ephemeral data.
"""
Ephemeral = 'ephemeral'
Persisted = 'persisted'

96
openedx/core/djangoapps/xblock/runtime/runtime.py
View File

@@ -1,8 +1,9 @@
"""
Common base classes for all new XBlock runtimes.
"""
from __future__ import annotations
import logging
from typing import Callable
from urllib.parse import urljoin # pylint: disable=import-error
import crum
@@ -13,12 +14,13 @@ from completion.services import CompletionService
from django.contrib.auth import get_user_model
from django.core.cache import cache
from django.core.exceptions import PermissionDenied
from functools import lru_cache # lint-amnesty, pylint: disable=wrong-import-order
from eventtracking import tracker
from opaque_keys.edx.keys import UsageKey, LearningContextKey
from web_fragments.fragment import Fragment
from xblock.core import XBlock
from xblock.exceptions import NoSuchServiceError
from xblock.field_data import SplitFieldData
from xblock.fields import Scope
from xblock.field_data import FieldData, SplitFieldData
from xblock.fields import Scope, ScopeIds
from xblock.runtime import KvsFieldData, MemoryIdManager, Runtime
from xmodule.errortracker import make_error_tracker
@@ -33,7 +35,9 @@ from common.djangoapps.track import views as track_views
from common.djangoapps.xblock_django.user_service import DjangoXBlockUserService
from lms.djangoapps.courseware.model_data import DjangoKeyValueStore, FieldDataCache
from lms.djangoapps.grades.api import signals as grades_signals
from openedx.core.types import User as UserType
from openedx.core.djangoapps.xblock.apps import get_xblock_app_config
from openedx.core.djangoapps.xblock.data import StudentDataMode
from openedx.core.djangoapps.xblock.runtime.blockstore_field_data import BlockstoreChildrenData, BlockstoreFieldData
from openedx.core.djangoapps.xblock.runtime.ephemeral_field_data import EphemeralKeyValueStore
from openedx.core.djangoapps.xblock.runtime.mixin import LmsBlockMixin
@@ -79,7 +83,16 @@ class XBlockRuntime(RuntimeShim, Runtime):
# This runtime can save state for users who aren't logged in:
suppports_state_for_anonymous_users = True
def __init__(self, system, user):
# Instance variables:
user: UserType | None
user_id: int | str | None
# dict of FieldData stores for our loaded XBlocks. Key is the block's scope_ids.
block_field_datas: dict[ScopeIds, FieldData | None]
# dict of FieldDataCache objects for XBlock with database-based user state
django_field_data_caches: dict[LearningContextKey, FieldDataCache]
def __init__(self, system: XBlockRuntimeSystem, user: UserType | None):
super().__init__(
id_reader=system.id_reader,
mixins=(
@@ -99,17 +112,17 @@ class XBlockRuntime(RuntimeShim, Runtime):
self.user_id = get_xblock_id_for_anonymous_user(user)
else:
self.user_id = self.user.id
self.block_field_datas = {} # dict of FieldData stores for our loaded XBlocks. Key is the block's scope_ids.
self.django_field_data_caches = {} # dict of FieldDataCache objects for XBlock with database-based user state
self.block_field_datas = {}
self.django_field_data_caches = {}
def handler_url(self, block, handler_name, suffix='', query='', thirdparty=False):
def handler_url(self, block, handler_name: str, suffix='', query='', thirdparty=False):
"""
Get the URL to a specific handler.
"""
if thirdparty:
log.warning("thirdparty handlers are not supported by this runtime for XBlock %s.", type(block))
url = self.system.handler_url(usage_key=block.scope_ids.usage_id, handler_name=handler_name, user=self.user)
url = self.system.handler_url(block.scope_ids.usage_id, handler_name, self.user)
if suffix:
if not url.endswith('/'):
url += '/'
@@ -122,7 +135,7 @@ class XBlockRuntime(RuntimeShim, Runtime):
def resource_url(self, resource):
raise NotImplementedError("resource_url is not supported by Open edX.")
def local_resource_url(self, block, uri):
def local_resource_url(self, block: XBlock, uri: str) -> str:
"""
Get the absolute URL to a resource file (like a CSS/JS file or an image)
that is part of an XBlock's python module.
@@ -132,7 +145,7 @@ class XBlockRuntime(RuntimeShim, Runtime):
absolute_url = urljoin(site_root_url, relative_url)
return absolute_url
def publish(self, block, event_type, event_data):
def publish(self, block: XBlock, event_type: str, event_data: dict):
""" Handle XBlock events like grades and completion """
special_handler = self.get_event_handler(event_type)
if special_handler:
@@ -140,7 +153,7 @@ class XBlockRuntime(RuntimeShim, Runtime):
else:
self.log_event_to_tracking_log(block, event_type, event_data)
def get_event_handler(self, event_type):
def get_event_handler(self, event_type: str) -> Callable[[XBlock, dict], None] | None:
"""
Return an appropriate function to handle the event.
@@ -156,7 +169,7 @@ class XBlockRuntime(RuntimeShim, Runtime):
return self.handle_completion_event
return None
def log_event_to_tracking_log(self, block, event_type, event_data):
def log_event_to_tracking_log(self, block: XBlock, event_type: str, event_data: dict) -> None:
"""
Log this XBlock event to the tracking log
"""
@@ -168,11 +181,11 @@ class XBlockRuntime(RuntimeShim, Runtime):
with tracker.get_tracker().context(event_type, log_context):
track_function(event_type, event_data)
def handle_grade_event(self, block, event):
def handle_grade_event(self, block: XBlock, event: dict):
"""
Submit a grade for the block.
"""
if not self.user.is_anonymous:
if self.user and not self.user.is_anonymous:
grades_signals.SCORE_PUBLISHED.send(
sender=None,
block=block,
@@ -184,7 +197,7 @@ class XBlockRuntime(RuntimeShim, Runtime):
grader_response=event.get('grader_response')
)
def handle_completion_event(self, block, event):
def handle_completion_event(self, block: XBlock, event: dict):
"""
Submit a completion object for the block.
"""
@@ -196,7 +209,7 @@ class XBlockRuntime(RuntimeShim, Runtime):
completion=event['completion'],
)
def applicable_aside_types(self, block):
def applicable_aside_types(self, block: XBlock):
""" Disable XBlock asides in this runtime """
return []
@@ -211,7 +224,7 @@ class XBlockRuntime(RuntimeShim, Runtime):
# Deny access to the inherited method
raise NotImplementedError("XML Serialization is only supported with BlockstoreXBlockRuntime")
def service(self, block, service_name):
def service(self, block: XBlock, service_name: str):
"""
Return a service, or None.
Services are objects implementing arbitrary other interfaces.
@@ -235,6 +248,8 @@ class XBlockRuntime(RuntimeShim, Runtime):
elif service_name == "completion":
return CompletionService(user=self.user, context_key=context_key)
elif service_name == "user":
if self.user is None:
raise RuntimeError("Cannot access user service when there is no user bound to the XBlock.")
if self.user.is_anonymous:
deprecated_anonymous_student_id = self.user_id
else:
@@ -244,13 +259,13 @@ class XBlockRuntime(RuntimeShim, Runtime):
self.user,
# The value should be updated to whether the user is staff in the context when Blockstore runtime adds
# support for courses.
user_is_staff=self.user.is_staff,
user_is_staff=self.user.is_staff, # type: ignore
anonymous_user_id=self.anonymous_student_id,
# See the docstring of `DjangoXBlockUserService`.
deprecated_anonymous_user_id=deprecated_anonymous_student_id
)
elif service_name == "mako":
if self.system.student_data_mode == XBlockRuntimeSystem.STUDENT_DATA_EPHEMERAL:
if self.system.student_data_mode == StudentDataMode.Ephemeral:
return MakoService(namespace_prefix='lms.')
return MakoService()
elif service_name == "i18n":
@@ -283,7 +298,7 @@ class XBlockRuntime(RuntimeShim, Runtime):
service = super().service(block, service_name)
return service
def _init_field_data_for_block(self, block):
def _init_field_data_for_block(self, block: XBlock) -> FieldData:
"""
Initialize the FieldData implementation for the specified XBlock
"""
@@ -292,10 +307,10 @@ class XBlockRuntime(RuntimeShim, Runtime):
student_data_store = None
elif self.user.is_anonymous:
# This is an anonymous (non-registered) user:
assert self.user_id.startswith("anon")
assert isinstance(self.user_id, str) and self.user_id.startswith("anon")
kvs = EphemeralKeyValueStore()
student_data_store = KvsFieldData(kvs)
elif self.system.student_data_mode == XBlockRuntimeSystem.STUDENT_DATA_EPHEMERAL:
elif self.system.student_data_mode == StudentDataMode.Ephemeral:
# We're in an environment like Studio where we want to let the
# author test blocks out but not permanently save their state.
kvs = EphemeralKeyValueStore()
@@ -324,7 +339,7 @@ class XBlockRuntime(RuntimeShim, Runtime):
Scope.preferences: student_data_store,
})
def render(self, block, view_name, context=None):
def render(self, block: XBlock, view_name: str, context: dict | None = None):
"""
Render a specific view of an XBlock.
"""
@@ -364,7 +379,7 @@ class XBlockRuntime(RuntimeShim, Runtime):
return fragment
def _lookup_asset_url(self, block, asset_path): # pylint: disable=unused-argument
def _lookup_asset_url(self, block: XBlock, asset_path: str): # pylint: disable=unused-argument
"""
Return an absolute URL for the specified static asset file that may
belong to this XBlock.
@@ -387,14 +402,11 @@ class XBlockRuntimeSystem:
class can be used with many different XBlocks, whereas each XBlock gets its
own instance of XBlockRuntime.
"""
STUDENT_DATA_EPHEMERAL = 'ephemeral'
STUDENT_DATA_PERSISTED = 'persisted'
def __init__(
self,
handler_url, # type: Callable[[UsageKey, str, Union[int, ANONYMOUS_USER]], str]
student_data_mode, # type: Union[STUDENT_DATA_EPHEMERAL, STUDENT_DATA_PERSISTED]
runtime_class, # type: XBlockRuntime
handler_url: Callable[[UsageKey, str, UserType | None], str],
student_data_mode: StudentDataMode,
runtime_class: type[XBlockRuntime],
):
"""
args:
@@ -403,8 +415,8 @@ class XBlockRuntimeSystem:
handler_url(
usage_key: UsageKey,
handler_name: str,
user_id: Union[int, str],
)
user: User | AnonymousUser | None
) -> str
student_data_mode: Specifies whether student data should be kept
in a temporary in-memory store (e.g. Studio) or persisted
forever in the database.
@@ -416,18 +428,17 @@ class XBlockRuntimeSystem:
self.runtime_class = runtime_class
self.authored_data_store = BlockstoreFieldData()
self.children_data_store = BlockstoreChildrenData(self.authored_data_store)
assert student_data_mode in (self.STUDENT_DATA_EPHEMERAL, self.STUDENT_DATA_PERSISTED)
assert student_data_mode in (StudentDataMode.Ephemeral, StudentDataMode.Persisted)
self.student_data_mode = student_data_mode
self._error_trackers = {}
def get_runtime(self, user):
def get_runtime(self, user: UserType | None) -> XBlockRuntime:
"""
Get the XBlock runtime for the specified Django user. The user can be
a regular user, an AnonymousUser, or None.
"""
return self.runtime_class(self, user)
def get_service(self, block, service_name):
def get_service(self, block, service_name: str):
"""
Get a runtime service
@@ -436,14 +447,5 @@ class XBlockRuntimeSystem:
XBlockRuntime.
"""
if service_name == 'error_tracker':
return self.get_error_tracker_for_context(block.scope_ids.usage_id.context_key)
return make_error_tracker()
return None # None means see if XBlockRuntime offers this service
@lru_cache(maxsize=32)
def get_error_tracker_for_context(self, context_key): # pylint: disable=unused-argument
"""
Get an error tracker for the specified context.
lru_cache makes this error tracker long-lived, for
up to 32 contexts that have most recently been used.
"""
return make_error_tracker()

3
openedx/core/types/user.py
View File

@@ -1,7 +1,8 @@
"""
Typing utilities for the User models.
"""
from typing import Union
import django.contrib.auth.models
User = django.contrib.auth.models.User
User = Union[django.contrib.auth.models.User, django.contrib.auth.models.AnonymousUser]

5
requirements/constraints.txt
View File

@@ -113,3 +113,8 @@ djangorestframework<3.15.0
# tests failing with greater version. Fix this in separate ticket.
pillow<10.0.0
# The version of django-stubs we can use depends on which Django release we're using
# 1.16.0 works with Django 3.2 through 4.1
django-stubs==1.16.0
djangorestframework-stubs==3.14.0 # Pinned to match django-stubs. Remove this when we can remove the above pin.

2
requirements/edx/development.in
View File

@@ -16,6 +16,8 @@
click # Used for perf_tests utilities in modulestore
django-debug-toolbar # A set of panels that display debug information about the current request/response
django-stubs # Typing stubs for Django, so it works with mypy
djangorestframework-stubs # Typing stubs for DRF
mypy # static type checking
pywatchman # More efficient checking for runserver reload trigger events
vulture # Detects possible dead/unused code, used in scripts/find-dead-code.sh

23
requirements/edx/development.txt
View File

@@ -358,6 +358,8 @@ django==3.2.20
# django-splash
# django-statici18n
# django-storages
# django-stubs
# django-stubs-ext
# django-user-tasks
# djangorestframework
# drf-jwt
@@ -576,6 +578,13 @@ django-storages==1.9.1
# -r requirements/edx/doc.txt
# -r requirements/edx/testing.txt
# edxval
django-stubs==1.16.0
# via
# -c requirements/edx/../constraints.txt
# -r requirements/edx/development.in
# djangorestframework-stubs
django-stubs-ext==4.2.2
# via django-stubs
django-user-tasks==3.0.0
# via
# -r requirements/edx/doc.txt
@@ -618,6 +627,10 @@ djangorestframework==3.14.0
# openedx-blockstore
# ora2
# super-csv
djangorestframework-stubs==3.14.0
# via
# -c requirements/edx/../constraints.txt
# -r requirements/edx/development.in
djangorestframework-xml==2.0.0
# via
# -r requirements/edx/doc.txt
@@ -1226,7 +1239,10 @@ multidict==6.0.4
# aiohttp
# yarl
mypy==1.4.1
# via -r requirements/edx/development.in
# via
# -r requirements/edx/development.in
# django-stubs
# djangorestframework-stubs
mypy-extensions==1.0.0
# via mypy
mysqlclient==2.2.0
@@ -1721,6 +1737,7 @@ requests==2.31.0
# analytics-python
# coreapi
# django-oauth-toolkit
# djangorestframework-stubs
# edx-bulk-grades
# edx-drf-extensions
# edx-enterprise
@@ -2012,6 +2029,7 @@ tomli==2.0.1
# -r requirements/edx/testing.txt
# build
# coverage
# django-stubs
# import-linter
# mypy
# pip-tools
@@ -2043,6 +2061,9 @@ typing-extensions==4.7.1
# asgiref
# astroid
# django-countries
# django-stubs
# django-stubs-ext
# djangorestframework-stubs
# faker
# fastapi
# grimp