Feat: replace drf yasg with drf spectacular - #39108
Faraz32123 wants to merge 8 commits into
Conversation
Sorry, something went wrong.
Converts @swagger_auto_schema to @extend_schema in the five modules that import drf_yasg directly, following the drf-spectacular migration guide: https://drf-spectacular.readthedocs.io/en/latest/drf_yasg.html Structured openapi.Schema objects become inline_serializer so they get named components in the generated schema; untyped ones become OpenApiTypes.OBJECT. openapi.Parameter becomes OpenApiParameter. The other four drf_yasg users go through edx-api-doc-tools and will be migrated with it.
Converts @apidocs.schema to @extend_schema across openedx/core, replacing the parameter helpers with OpenApiParameter and string response descriptions with OpenApiResponse. bookmarks/serializers.py inlines is_schema_request, which has no drf-spectacular equivalent, and extends it to recognise drf-spectacular's swagger_fake_view alongside drf-yasg's format=openapi.
Converts @apidocs.schema and @Schema to @extend_schema across the lms app, excluding instructor. Parameter helpers become OpenApiParameter and string response descriptions become OpenApiResponse. discussion/rest_api/views.py also drops its remaining direct drf_yasg import, which was interleaved with the apidocs decorators.
Converts the 26 @apidocs.schema decorators in the instructor v1 and v2 APIs to @extend_schema. The course_id, problem, and exam_id path parameters were repeated verbatim across 29 decorators; those are now module-level constants.
Converts the remaining @apidocs.schema decorators across contentstore and modulestore_migrator to @extend_schema. The three class-level @apidocs.schema_for decorators become @extend_schema_view, splitting each docstring into summary and description as schema_for did. Files that already imported drf-spectacular for the FC-0118 work have their import lines merged rather than duplicated.
Six serializers subclass BaseSerializer, which has no `fields` attribute, so drf-spectacular raises AttributeError when generating a schema that covers them. Each extension declares the type its serializer produces. Registered from CommonInitializationConfig.ready() so they load in both services regardless of which schema is being generated.
Replaces make_docs_urls with SpectacularAPIView, SpectacularSwaggerView and SpectacularRedocView, preserving the swagger.json, swagger.yaml, api-docs/ and swagger/ routes and their URL names. The UI views reverse their schema URL without arguments, so api-docs/schema/ is registered alongside the format-suffixed routes. /api-docs serves the full API surface via custom_settings, leaving SPECTACULAR_SETTINGS to the narrower Authoring and Enrollment schemas the SDK consumes. Also removes drf_yasg from INSTALLED_APPS, drops SWAGGER_SETTINGS, and converts the docs security definitions to OpenAPI 3 form. `make swagger` now runs `manage.py lms spectacular`, since generate_swagger came from drf_yasg; docs_settings applies the same unfiltered configuration as /api-docs so the generated file still covers the whole surface. edx-api-doc-tools and drf-yasg remain installed as transitive dependencies of openedx-authz and django-user-tasks respectively.
| urlpatterns += [ | ||
| re_path( | ||
| r'^swagger\.(?P<format>json|yaml)$', | ||
| SpectacularAPIView.as_view(custom_settings=get_api_docs_settings()), |
There was a problem hiding this comment.
These endpoints lose their server-side cache.
openedx/envs/common.py has, with the comment "How long to cache OpenAPI schemas and UI, in seconds":
OPENAPI_CACHE_TIMEOUT = 60 * 60Nothing in lms/envs/production.py or cms/envs/production.py overrides it — only devstack and test set it to 0 — so production runs with a 1-hour cache today. edx-api-doc-tools fed that value into SchemaView.as_cached_view, which wraps the view in vary_on_headers("Cookie", "Authorization") + cache_page(timeout) (drf_yasg/views.py:134-164).
SpectacularAPIView has no caching of any kind, so after this change every request to /swagger.json, /swagger.yaml, and the new /api-docs/schema/ regenerates the entire schema. These are public and unauthenticated (SERVE_PERMISSIONS defaults to AllowAny, which matches the old permission_classes=(AllowAny,)), and the schema being generated is also roughly twice as large as before (see my note on openedx/core/apidocs.py).
It also leaves OPENAPI_CACHE_TIMEOUT as a dead setting that still claims to do something.
Could we keep using the existing setting — wrapping the two SpectacularAPIView routes in cache_page(settings.OPENAPI_CACHE_TIMEOUT)? Same applies to cms/urls.py.
Sorry, something went wrong.
There was a problem hiding this comment.
Good catch, I'll use the existing setting.
Sorry, something went wrong.
| """ | ||
| Build the ``/api-docs`` schema settings, adding contact details if available. | ||
|
|
||
| ``API_ACCESS_MANAGER_EMAIL`` is an LMS-only setting, so it is included only |
There was a problem hiding this comment.
Small correction: API_ACCESS_MANAGER_EMAIL isn't LMS-only. It's defined in openedx/envs/common.py (~line 2721), which both lms/envs/common.py and cms/envs/common.py star-import — the old code here read it unconditionally at import time from a module cms/urls.py imported, which wouldn't have worked otherwise.
The getattr(..., None) guard is harmless, but the docstring should probably just say the contact is included when the setting is present, without the LMS-only claim.
Sorry, something went wrong.
| ``AttributeError`` when it tries to walk them. Each extension below declares | ||
| the type its serializer actually produces. | ||
|
|
||
| The extensions self-register on import; ``lms.lib.spectacular`` and |
There was a problem hiding this comment.
This paragraph doesn't match the implementation — neither lms/lib/spectacular.py nor cms/lib/spectacular.py imports this module. The only importer is openedx/core/djangoapps/common_initialization/apps.py in ready(), which is the right place.
Looks like a leftover from an earlier approach; worth repointing the docstring at the AppConfig so the next reader can find the registration site.
Sorry, something went wrong.
|
|
||
| Schema generators set a swagger_fake_view attribute on the view; that is | ||
| the drf-spectacular-compatible signal. ``format=openapi`` is drf-yasg's | ||
| convention, kept while it still serves ``/api-docs``. |
There was a problem hiding this comment.
The swagger_fake_view check is correct — drf-spectacular sets it on the view (generators.py:141) before build_mock_request builds the DRF Request, so parser_context['view'] resolves to that view.
The docstring is stale though: this PR is what stops drf-yasg serving /api-docs, so "kept while it still serves /api-docs" no longer holds and the format=openapi branch is dead as far as the platform is concerned. Either drop the fallback or reword to say it's kept for out-of-tree callers.
Sorry, something went wrong.
| @@ -2149,10 +2148,6 @@ | |||
|
|
|||
| ######################### Django Rest Framework ######################## | |||
There was a problem hiding this comment.
Nit: removing SWAGGER_SETTINGS leaves this banner with nothing under it. Worth deleting the header too, since the drf-spectacular block below has its own.
Sorry, something went wrong.
- restore server-side caching on the OpenAPI schema endpoints, which edx-api-doc-tools provided via SchemaView.as_cached_view - point the api-docs test at /api-docs/schema/ so it exercises schema generation again, and add the CMS equivalent - serve Swagger UI and ReDoc assets from drf-spectacular-sidecar instead of drf-spectacular's unpinned jsdelivr CDN defaults - correct the /api-docs comment: edx-api-doc-tools was /api/-only, so this widens the documented surface rather than being "the opposite" - drop the LMS-only claim about API_ACCESS_MANAGER_EMAIL, which lives in openedx/envs/common.py and is shared by both services - point the schema_extensions docstring at CommonInitializationConfig, the actual registration site - remove the dead format=openapi branch from is_schema_request, since nothing in the platform serves drf-yasg any more - delete the now-empty Django Rest Framework banner in lms/envs/common.py
| # Schema generation is expensive and these endpoints are public, so both schema | ||
| # routes are cached for OPENAPI_CACHE_TIMEOUT, as edx-api-doc-tools did via | ||
| # SchemaView.as_cached_view. | ||
| _apidocs_schema_view = cache_page(settings.OPENAPI_CACHE_TIMEOUT)( |
There was a problem hiding this comment.
Sorry, I didn't point this out before, but it turns out that the document here might be too big for the cache as is. I measured the rendered response at head 4bf390f6 under docs.docs_settings: /api-docs/schema/ and /swagger.yaml pickle to 1,253,795 bytes and /swagger.json to 1,694,003 bytes, both over memcached's 1MiB default item limit. CACHES['default'] is PyMemcacheCache with ignore_exc: True (openedx/envs/common.py:362-373), so the failed set is swallowed and Django deletes the key.
Cache the compressed body instead of letting cache_page pickle the whole Response, the way openedx/core/lib/edx_api_utils.py:78-121 already does:
# openedx/core/apidocs.py
import logging
from django.core.cache import cache
from django.http import HttpResponse
from django.utils.cache import add_never_cache_headers
from edx_django_utils.cache import get_cache_key
from openedx.core.lib.cache_utils import zpickle, zunpickle
log = logging.getLogger(__name__)
def cached_schema_view():
"""
Build the ``/api-docs`` schema view, caching the rendered document compressed.
``cache_page`` stores the pickled ``Response``, which for this schema is ~1.7MB and so
over memcached's default 1MB item limit: the ``set`` fails and nothing is ever cached.
Storing the zlib-compressed body instead brings it to ~110KB. The document is identical
for every requester, so the key covers only the path and the negotiated representation.
``drf_spectacular.views`` is imported inside the function, not at module scope: this
module is imported from the settings, and importing DRF views that early freezes
``api_settings`` before ``DEFAULT_SCHEMA_CLASS`` is set, which makes schema generation
fail with ``Incompatible AutoSchema used on View``.
"""
from drf_spectacular.views import SpectacularAPIView
view = SpectacularAPIView.as_view(custom_settings=get_api_docs_settings())
def _response(content_type, body):
response = HttpResponse(body, content_type=content_type)
add_never_cache_headers(response)
return response
def schema_view(request, *args, **kwargs):
if not settings.OPENAPI_CACHE_TIMEOUT:
return view(request, *args, **kwargs)
cache_key = get_cache_key(
resource='apidocs-schema',
path=request.get_full_path(),
accept=request.META.get('HTTP_ACCEPT', ''),
) + '.zpickled'
cached = cache.get(cache_key)
if cached:
try:
content_type, body = zunpickle(cached)
except Exception: # pylint: disable=broad-except
log.warning("Data for cache is corrupt for cache key %s", cache_key)
cache.delete(cache_key)
else:
return _response(content_type, body)
response = view(request, *args, **kwargs).render()
if response.status_code != 200:
return response
content_type = response['Content-Type']
cache.set(cache_key, zpickle((content_type, response.content)),
settings.OPENAPI_CACHE_TIMEOUT)
return _response(content_type, response.content)
return schema_viewThen here, dropping the cache_page and vary_on_headers imports:
_apidocs_schema_view = cached_schema_view()I ran the above on 4bf390f6 against a local memory cache: largest stored entry 109,504 bytes, 10.4% of the limit, hits return in 0.003s against 0.33 to 0.76s to generate, and the bodies are byte identical to the uncached ones. add_never_cache_headers restores the no-store drf-yasg got from deferred_never_cache (drf_yasg/views.py:134-140), which is why these routes currently answer Cache-Control: max-age=3600. Dropping vary_on_headers is safe because nothing in the document varies by user: SERVE_PUBLIC defaults to True, SERVE_* cannot be set through custom_settings, and API_DOCS_SETTINGS sets 'SERVERS': [] so no servers block is emitted (drf_spectacular/plumbing.py:526-527).
Same block in cms/urls.py.
Sorry, something went wrong.
Reviewers
At least 1 approving review is required to merge this pull request.
This PR can be merged after this schema generation PR: #39025
Replace drf-yasg with drf-spectacular
Follow-up to the FC-0118 API standardization work. Migrates all platform API
documentation off drf-yasg and edx-api-doc-tools onto drf-spectacular,
including the /api-docs site itself.
Slack Discussion thread, the goal was to drop both dependencies as part
of the drf-spectacular conversion, replacing the edx-api-doc-tools /api-docs
endpoint with the drf-spectacular equivalent.
What changed
~400 decorator call sites across 43 files, following the
drf-yasg migration guide:
openapi.Parameter → OpenApiParameter
OpenApiTypes where they didn't
SpectacularRedocView
Commits
Reviewable one at a time, each independently handled:
Three things worth knowing
The dependencies don't actually disappear. edx-api-doc-tools is still
required by openedx-authz, and drf-yasg by django-user-tasks. Both remain
in base.txt as transitive dependencies. What this PR removes is the platform's
own dependency on them — pyproject.toml, INSTALLED_APPS, and every import.
Fully dropping them needs those upstream packages to migrate first.
Commit 6 exists because /api-docs is unfiltered. Six serializers subclass
BaseSerializer, which has no fields attribute, so drf-spectacular raised
AttributeError when generating a schema covering the whole surface. This never
surfaced before because the existing drf-spectacular schemas
(/authoring-api/, /lms-api/) are filtered down to a handful of endpoints.
Each serializer now has an OpenApiSerializerExtension declaring the type it
actually produces.
/api-docs documents more than it used to. edx-api-doc-tools'
ApiSchemaGenerator kept only paths under /api/; drf-spectacular documents
every DRF endpoint in the service — 633 LMS paths versus the 293 in the
committed docs/lms-openapi.yaml. That file also becomes an OpenAPI 3 document
rather than Swagger 2.0, with untrimmed paths. Flagging it so the widening is a
recorded decision rather than a side effect.
Verification
Tested against a running Tutor instance:
The last two matter most: /api-docs uses custom_settings rather than the
global SPECTACULAR_SETTINGS, so the narrow SDK-facing schemas are untouched —
same paths, same prefix trimming. The SDK's generated client is unaffected.
Swagger UI and ReDoc confirmed rendering on both services. swagger.json,
swagger.yaml, api-docs/ and swagger/ keep their existing paths and URL
names; api-docs/schema/ is new, because the Swagger and ReDoc views reverse
their schema URL without arguments.
Schema caching is preserved: with OPENAPI_CACHE_TIMEOUT at its 1-hour default,
the first request to /api-docs/schema/ took 5.69s and the second 0.08s.
Notes for reviewers
string responses and 46 parameters, so the repetitive conversions were
scripted. Spot-checking won't catch a systematic miss — I verified by
extracting every response and parameter from both versions and diffing the
sets. Same method used on commits 1–3 and 5 to confirm nothing was dropped.
(type: basic) to OpenAPI 3 (type: http, scheme: basic). This is the one
change I couldn't exercise locally — it only takes effect in the Sphinx docs
build.
stay self-hosted and version-pinned as the drf-yasg bundles were, rather than
drf-spectacular's default unpinned @latest CDN URLs. This is a new
dependency — a uv sync or container rebuild is needed on this branch.
unresolvable authenticators, two CourseEnrollment components with clashing
names). They don't block generation and are out of scope here.