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

Do not use a leading dot to specify SwaggerRenderer formats #667

Open
wants to merge 1 commit into
base: 1.21.x
Choose a base branch
from

Conversation

brianhelba
Copy link
Contributor

@brianhelba brianhelba commented Oct 29, 2020

DRF expects that custom Renderers use a simple string for the format: https://www.django-rest-framework.org/api-guide/renderers/#custom-renderers

This is also required to support content negotiation via DRF's native mechanisms, particularly format_suffix_patterns: https://www.django-rest-framework.org/api-guide/format-suffixes/

Note that all the other DRF-YASG renderers (including OpenAPIRenderer and SwaggerUIRenderer) already do the correct thing. This change just updates the SwaggerJSONRenderer and SwaggerYAMLRenderer to be consistent and correct.

DRF expects that custom Renderers use a simple string for the "format":
https://www.django-rest-framework.org/api-guide/renderers/#custom-renderers

This is also required to support content negotiation via DRF's native
mechanisms, particularly "format_suffix_patterns":
https://www.django-rest-framework.org/api-guide/format-suffixes/

Note that all the other DRF-YASG renderers (including OpenAPIRenderer
and SwaggerUIRenderer) already do the correct thing. This change
just updates the SwaggerJSONRenderer and SwaggerYAMLRenderer to
be consistent and correct.
@brianhelba
Copy link
Contributor Author

Unfortunately, this is not compatible with the existing URL configuration that DRF-YASG's docs have been instructing users to configure. If this is released as-is, it would be a breaking change.

I'm open to ways to working in a compatibility mechanism, if it's agreed that this is a positive change.

@JoelLefkowitz JoelLefkowitz changed the base branch from master to 1.21.x July 17, 2022 17:18
@JoelLefkowitz JoelLefkowitz added 1.21.x Release target in 1.21.x help wanted Help wanted Incomplete Not ready for merging and removed 1.21.x Release target in 1.21.x labels Jul 17, 2022
@creyD
Copy link

creyD commented Jul 19, 2022

@JoelLefkowitz Maybe it would be best to apply semantic versioning here? As it is a breaking change this would belong in version 2.x.x. Maybe there are other changes that are also breaking, these all could be added in 2.x including OpenAPI 3 support.

@JoelLefkowitz
Copy link
Collaborator

@creyD i think it's best to roll everything out compatibly and keep the public interfaces consistent with what the bulk of the user base are expecting. To support this url pattern we should extend the customisation support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Help wanted Incomplete Not ready for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants