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 #1050

Closed
wants to merge 3 commits into from

Conversation

afuetterer
Copy link
Member

@afuetterer afuetterer commented Jul 13, 2024

Description

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

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

@afuetterer afuetterer marked this pull request as draft July 13, 2024 07:26
@afuetterer afuetterer linked an issue Jul 13, 2024 that may be closed by this pull request
@afuetterer afuetterer added type:maintenance dependencies Pull requests that update a dependency file python Pull requests that update Python code labels Jul 13, 2024
@afuetterer afuetterer changed the title feat: replace django-rest-swagger with drf-spectacular feat: replace django-rest-swagger with drf-spectacular Jul 15, 2024
Copy link
Member

@jochenklar jochenklar left a comment

Choose a reason for hiding this comment

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

Thanks, it looks really nice. I have some questions left.


# TODO: maybe generate the schema.yml as part of the build process?

urlpatterns = [
Copy link
Member

Choose a reason for hiding this comment

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

I think either the 'swagger' view should be the root of those urlpatterns (to emulate the current behavior) or we add a small template with links to the 3 endpoints.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure thing. Will update the url patterns.

At the moment the schema view generates the schema at runtime. drf-spectacular can generate the schema per cli. Should the schema be served from static files instead? What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

We could create the file before the release and put it in rdmo/share.

Copy link
Member

Choose a reason for hiding this comment

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

And as with the migration, it would be nice if there is a check in the CI if the file is up to date. The build itself should be included into the new build script: https://github.com/rdmorganiser/rdmo/blob/2.3.0/rdmo/core/management/commands/build.py

@afuetterer afuetterer force-pushed the 698-drf-spectacular branch from c459a3b to ad33ced Compare July 19, 2024 15:08
@jochenklar jochenklar added this to the RDMO 2.3.0 milestone Jul 26, 2024
@jochenklar jochenklar deleted the branch rdmorganiser:2.3.0 September 20, 2024 08:17
@jochenklar jochenklar closed this Sep 20, 2024
@MyPyDavid
Copy link
Member

should this PR be re-opened and set to 2.3.0 or we need to make a new PR?

@jochenklar
Copy link
Member

Yes, @afuetterer , can you open the PR again? We would rebase to 2.3.0 and test it again.

@afuetterer
Copy link
Member Author

Yes, will do.

@afuetterer
Copy link
Member Author

I can not reopen.

@MyPyDavid MyPyDavid reopened this Feb 21, 2025
@MyPyDavid MyPyDavid changed the base branch from dev-2.2.0 to 2.3.0 February 21, 2025 07:59
@MyPyDavid
Copy link
Member

MyPyDavid commented Feb 21, 2025

ah ok, it needed the old target branch (dev-2.2.0) to be present (before anybody is able to re-open it) . I guess I will open a duplicate PR of this one with your branch pushed sourced to this rdmorganiser repo.

@MyPyDavid
Copy link
Member

MyPyDavid commented Feb 21, 2025

ok, let's continue this then in #1248! 🙏

@MyPyDavid MyPyDavid closed this Feb 21, 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 python Pull requests that update Python code status:hold type:maintenance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace django-rest-swagger with drf-spectacular
3 participants