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

Add response content_type(s) #19

Merged

Conversation

philippeauriach
Copy link
Contributor

OpenAPI let responses have multiple response_types, as per https://swagger.io/docs/specification/describing-responses/

This PR adds:

  • a new content_type in the response object, with a default to application/json (the default used before)
  • the ability to pass several responses for the same response code, BUT with a different content_type (passing two responses with the same code + content type would raise an error, as we did before when passing two responses with the same code)

The only "issue" I see is that the response description is taken from the first response. I initially wanted to modify the response object to

class Response:
-    def __init__(self, model: type, code: int = 200, description: str = "Success", content_type: str = DEFAULT_CONTENT_TYPE):
+    def __init__(self, code: int = 200, content, description: str = "Success"):
-        self.model = model
        self.code = code
        self.description = description
+        self.content = content # content would be an array of {content_types: str, model: type}

But I feel that this would change too much the current behavior, so I preferred to "duplicate" the "description" field. I am of course open to discussion on this point !

@philippeauriach
Copy link
Contributor Author

Hello @jakemwood, what is the process to get a pull request to review ? Sorry to tag you, but as you're the one who reviewed my previous pull request... thanks!

Copy link
Contributor

@jakemwood jakemwood left a comment

Choose a reason for hiding this comment

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

Sorry for the delay here, this seems to make sense! I'll get this in.

@jakemwood jakemwood merged commit da021dd into TestBoxLab:main Dec 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants