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

Defining multiple responses #15

Closed
fbergroth opened this issue Oct 11, 2018 · 7 comments
Closed

Defining multiple responses #15

fbergroth opened this issue Oct 11, 2018 · 7 comments

Comments

@fbergroth
Copy link

I'd like to define multiple response codes for a view (201 for created, 200 for updated).

It seems that decorating a view twice will always return 200:

    @thing_bp.response(ThingSchema, code=200, description='Thing was updated')
    @thing_bp.response(ThingSchema, code=201, description='Thing was created')
    def post(...)

Is there a way around this?

Thanks

@fbergroth
Copy link
Author

@thing_bp.doc(responses={...}) seems to work well.

@lafrech
Copy link
Member

lafrech commented Oct 15, 2018

The framework is based on a few opinionated choices. The way we use it, the view function can only return the "normal" response, the one for which the status code is specified in the @response decorator.

Then there can be errors that interrupt the normal flow and generate an abort(another_status_code).

Indeed, you can document other responses using @doc. It is precisely meant to add documentation that is not grabbed generated from the code. BTW, I'd like to make it easier to document error responses. I'm working on apispec to make this easier by allowing to declare reusable response components.

However, calling twice the @response decorator is not supported. I'm pretty sure that would end up in a mess. I could add a check and raise an Exception if someone attempts to do that.

So you can document the two error codes using @doc, but how do you manage to actually return those error codes?

What is your use case? I suppose you're allowing PUT to create entities. Whether this is good practice is debated AFAIK. This is not something we do so we never tried to support it. We only use POST to create and PUT to update.

@fbergroth
Copy link
Author

The framework is based on a few opinionated choices. The way we use it, the view function can only return the "normal" response, the one for which the status code is specified in the @response decorator.

Yeah, it's been a pleasure to use it. This is just an edge case.

What I'm doing may not be considered strictly restful. My resource is a list of strings. Posting a list of strings once to /resource/<key>/ will create it, and subsequent calls will extend it. So it's not idempotent, hence the post instead of put.

So you can document the two error codes using @doc, but how do you manage to actually return those error codes?

It's actually success codes. I just decorate my view with bp.arguments and bp.doc (and not bp.response), and return a normal flask response object from the view.

Indeed, you can document other responses using @doc. It is precisely meant to add documentation that is not grabbed generated from the code. BTW, I'd like to make it easier to document error responses. I'm working on apispec to make this easier by allowing to declare reusable response components.

Cool! I would have liked another decorator that takes the same args as response (plus mimetype maybe), but just updates func._apidoc, but maybe you have other plans in mind.

@lafrech
Copy link
Member

lafrech commented Oct 15, 2018

What I'm doing may not be considered strictly restful. My resource is a list of strings. Posting a list of strings once to /resource/<key>/ will create it, and subsequent calls will extend it. So it's not idempotent, hence the post instead of put.

Yes, this is not the common case. I won't comment on restfulness. What happens if you GET /resource/<key>/ before the first POST? 404 or empty list? In your case I'd be tempted to use PATCH and always return 200. But I'd still consider the whole thing smelly.

I have more or less similar case in an app and I use PATCH and return 204 because I don't return the result.

It's actually success codes.

I understand. What I meant is that I stick to the "only one possible successful code" approach and I'd like to add better means to document error codes.

I just decorate my view with bp.arguments and bp.doc (and not bp.response), and return a normal flask response object from the view.

OK, I get it. But then you're far off beaten paths. It works but not using @response is not "supported". @response does not only make the response, it also manages ETag (which I suppose you don't use on this resource, if at all), it adds pagination header, and it could do other stuff.

I would have liked another decorator that takes the same args as response (plus mimetype maybe), but just updates func._apidoc, but maybe you have other plans in mind.

It's not really the direction I'd like to take. I'd like to keep the doc inferred from the code as much as possible. When needed, use @doc.

Regarding mimetype, I suppose you're referring to application/json. Currently, it is hardcoded, and I'd like to make it a parameter, but it is a bit more complicated than I expected so I left it for later.

@fbergroth
Copy link
Author

What happens if you GET /resource/<key>/ before the first POST?

It'll be a 404.

But I'd still consider the whole thing smelly.

Sure. I guess the alternative is post /resource/<key>/ to create it, and post/patch to /resource/<key>/append to append to it. But let's not delve into this too deeply.

If you want to force users to have a single success response, then I'll accept it.

Regarding far off beaten paths and mimetypes, I have another view that responds with an image, where I'm also using not using bp.response:

    @bp.arguments(ArgsSchema, as_kwargs=True)
    @bp.doc(
        responses={
            '200': {
                'description': 'An image',
                'content': {
                    'image/png': {
                        'schema': {
                            'type': 'string',
                            'format': 'binary',
                        }
                    }
                }
            }
        }
    )
    def post(self, ...):
        ...
        return send_file(img, mimetype='image/png')

I saw no other way to do it right now.

@lafrech
Copy link
Member

lafrech commented Oct 15, 2018

If you want to force users to have a single success response, then I'll accept it.

I'm open to changes. It's a tradeoff between relevance of the use case and difficulty of the change. I don't see any easy way to change that.

Regarding the mime types, I was thinking of a way to globally replace the serializer, for instance to replace json with XML. This is easy, but making the doc consistent revealed more complicated than I thought.

I think you did the right thing about the image use case. Improving @response to allow you to specify another mime type is only one part of the issue. You'd need to be able to replace the serializer (normally json), which we could allow at Blueprint level, but I don't see how to do that at resource level. Overall, not using @response for file UL/DL resources is probably the best way. In fact that's what we do. And we document the type manually using @doc.

@fbergroth
Copy link
Author

Yeah, it seems to be very tricky.

Maybe you can check (in the response-wrapper) if the view returned a flask response, and if so, skip serialization. That could be a way for users to override the serializer at the resource level, atleast until there is another way to do it.

Just to clarify:

# Build response
if isinstance(resp, Response):
    pass  # do not serialize
elif mimetype == 'application/json': # default value
    resp = jsonify(self._prepare_response_content(result_dump))
else:
    raise SomeError

I guess you'd would have a mapping from mimetype to serialization function to make it proper.

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

No branches or pull requests

2 participants