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

feat: replace django-rest-swagger with drf-spectacular #1248

Draft
wants to merge 3 commits into
base: 2.3.0
Choose a base branch
from

Conversation

MyPyDavid
Copy link
Member

@MyPyDavid MyPyDavid commented Feb 21, 2025

copied from #1050, rebased onto 2.3.0 > dependency-updates > drf-spectacular

Description

Duplicate of PR: #1050
Related issue: #698

Tasks

  • decide about NOTICE
  • remove swagger.scss
  • decide about the schema
  • decide about the purpose of the static schema in https://github.com/rdmorganiser/rdmo-openapi, which is 6 years old
  • handle warning and errors in schema generation
  • add tests
  • delete temporary django-rest-swagger.yaml schema file
  • remove ignore warnings from pytest config
  • squash commits

drf-spectacular warnings to be fixed

1. /rdmo/conditions/

Serializers: serializers/v1.py

  • ConditionViewSet > ConditionSerializer: Unable to resolve type hint for function get_model. Consider using a type hint or @extend_schema_field. Defaulting to string.

Viewsets: viewsets.py

  • ConditionViewSet: Could not derive type of path parameter "var" because model rdmo.conditions.models.Condition contained no such field. Consider annotating parameter with @extend_schema. Defaulting to "string".

2. /rdmo/core/

Serializers: serializers.py

  • RelationViewSet > ChoicesSerializer: Unable to resolve type hint for functions get_id and get_text.
  • GroupViewSet > GroupSerializer: Identical component names "Group" found in rdmo.core.serializers.GroupSerializer and rdmo.accounts.serializers.v1.GroupSerializer. Consider renaming one.
  • SitesViewSet > SiteSerializer: Identical component names "Site" found in rdmo.core.serializers.SiteSerializer and rdmo.accounts.serializers.v1.SiteSerializer. Consider renaming one.

Viewsets: viewsets.py

  • SettingsViewSet: Exception raised while getting serializer. Ensure get_serializer_class() is not returning None and get_queryset() works.
  • TemplatesViewSet: Similar issue as above.

3. /rdmo/domain/

Serializers: serializers/v1.py

  • AttributeViewSet > AttributeListSerializer: Unable to resolve type hints for get_model and is_leaf_node.
  • AttributeViewSet > AttributeSerializer: Unable to resolve type hints for get_model, get_tasks, and get_attributes.

Viewsets: viewsets.py

  • AttributeViewSet: Could not derive type of path parameter "var" due to missing field in rdmo.domain.models.Attribute.

4. /rdmo/management/

Viewsets: viewsets.py

  • ImportViewSet, MetaViewSet, UploadViewSet: Unable to guess serializer. Consider using GenericAPIView or explicitly adding serializer_class.

5. /rdmo/options/

Serializers: serializers/v1/option.py

  • OptionViewSet > OptionSerializer: Unable to resolve type hints for get_model, text, help, view_text, label, and get_warning.

Serializers: serializers/v1/optionset.py

  • OptionSetViewSet > OptionSetSerializer: Unable to resolve type hints for get_model and get_condition_uris.

Viewsets: viewsets.py

  • OptionViewSet, OptionSetViewSet: Path parameter "var" could not be derived due to missing fields in rdmo.options.models.Option and rdmo.options.models.OptionSet.

6. /rdmo/projects/

Serializers: serializers/v1/__init__.py

  • MembershipViewSet > MembershipSerializer: Identical component names "Membership" found in rdmo.projects.serializers.v1.MembershipSerializer and rdmo.accounts.serializers.v1.MembershipSerializer. Consider renaming one.
  • ProjectViewSet > ProjectSerializer: Unable to resolve type hint for catalog_uri.
  • ProjectViewSet > ProjectSerializer > UserSerializer: Identical component names "User" found in rdmo.projects.serializers.v1.UserSerializer and rdmo.accounts.serializers.v1.UserSerializer. Consider renaming one.

Viewsets: viewsets.py

  • MembershipViewSet: Failed to obtain model due to missing queryset. Fix by setting queryset = Model.objects.none() or using @extend_schema.

7. /rdmo/questions/

Serializers: serializers/v1/catalog.py

  • CatalogViewSet > CatalogSerializer: Identical component names "Catalog" found in rdmo.questions.serializers.v1.catalog.CatalogSerializer and rdmo.projects.serializers.v1.overview.CatalogSerializer. Consider renaming one.
  • Unable to resolve type hints for get_model, title, help, and get_warning.

Viewsets: viewsets.py

  • CatalogViewSet, PageViewSet, QuestionViewSet, QuestionSetViewSet, SectionViewSet: Path parameter "var" could not be derived due to missing fields.

8. /rdmo/tasks/

Serializers: serializers/v1.py

  • TaskViewSet > TaskSerializer: Unable to resolve type hints for get_model, title, text, get_warning, and get_condition_uris.

Viewsets: viewsets.py

  • TaskViewSet: Path parameter "var" could not be derived due to missing field.

9. /rdmo/views/

Serializers: serializers/v1.py

  • ViewViewSet > ViewSerializer: Unable to resolve type hints for get_model, title, help, and get_warning.

Viewsets: viewsets.py

  • ViewViewSet: Path parameter "var" could not be derived due to missing field.

10. General API Operation ID Collisions

  • Fix operation ID collisions in endpoints related to:
    • conditions_conditions_export_retrieve
    • domain_attributes_export_retrieve
    • options_options_export_retrieve
    • options_optionsets_export_retrieve
    • questions_catalogs_export_retrieve
    • questions_pages_export_retrieve
    • questions_questions_export_retrieve
    • questions_questionsets_export_retrieve
    • questions_sections_export_retrieve
    • tasks_tasks_export_retrieve
    • views_views_export_retrieve

Types of Changes

Mix between bug fix (deprecated library is replaced), new feature (schema is enhanced) and maybe breaking change (schema is different from before).

Checklist

  • I have read the contributor guide.
  • My code follows the code style of this project.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Refs

@MyPyDavid MyPyDavid changed the base branch from main to dependency-updates February 21, 2025 08:27
@MyPyDavid
Copy link
Member Author

Something was discussed about the urlpatterns:
#1050 (comment)

@MyPyDavid MyPyDavid added type:maintenance status:work-in-progress dependencies Pull requests that update a dependency file labels Feb 21, 2025
@jochenklar jochenklar added this to the RDMO 2.3.0 milestone Feb 21, 2025
@jochenklar
Copy link
Member

About the url patterns: I think they are fine, I would move the schema generation to /api/v1/openapi/ and leave the root /api/v1 empty or add a minimal template. With the 3 links.

@jochenklar
Copy link
Member

I managed to fix a lot of warnings by adding return value type hints to (a) model properties and (b) SerializerMethodField methods. I am still not a fan, but I can live with those. I think its better than to inlcude the decorator from drf-spectacular. It could even be an optional dependency. For the remaining problems this could be the way: https://drf-spectacular.readthedocs.io/en/latest/customization.html#step-5-extensions

I think all of this should be done after at least the release candidate and we live with the warnings for now.

@MyPyDavid MyPyDavid changed the base branch from dependency-updates to 2.3.0 February 24, 2025 09:59
@MyPyDavid MyPyDavid removed this from the RDMO 2.3.0 milestone Feb 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file status:work-in-progress type:maintenance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants