Support multiple status codes and schemas with responds

[apryor6]

Currently, only a single responds decorator may be provided, optionally with a status_code. Support for multiple schemas corresponding to different status codes would be straightforward to implement by inspecting the return type and mapping the status_code to the schema, if it is available.

The current functionality for ignoring pre-made Flask Responses would need to be modified to only ignore those for which it does not have a corresponding Schema.

The status code of the response can come from any of the normal Flask means (i.e. as a data, code, headers tuple, via make_response, etc)

[apogiatzis]

Firstly thanks for this handy package!

That would be a really nice feature! If I find some spare time I will try to open a PR for this

For the time being here is my workaround in case you want to achieve this with Marshmallow schemas:

@ns.route("/testroute")
class TestResource(Resource):
    """Testing Endpoint"""

    @ns.response(200, "Success", for_swagger(model_schema, ns))
    @ns.response(404, "NotFound",for_swagger(error_schema, ns))
    def get(self):
        """Return something"""
        .....
        return obj

where ns is the Namespace or Api, and the schemas are Marshmallow schemas

[odesenfans]

I concur, the handling of the various status codes surprises me. I see at least 2 different use cases:

1. Different schemas for different situations (ex: success, failure)
2. Different status codes for the same schema (ex: in a PUT request, return 201 on creation but 200 or 204 on update).

For 1, the type could be inferred in the responds decorator for simple cases. However, it's not easy for non-trivial schemas, ex: returning an object from the DB with an auto-generated marshmallow schema.
For 2, using the same schema, I don't see a proper solution. flask-accepts can't really make the difference between return response_content, status_code and a tuple schema.

Best solution could actually be to remove the status_code parameter from the responds decorator entirely...

[odesenfans]

After giving it a bit more thought, I've come up with the following solution:

1. Force the return value of endpoints decorated with responds to be a tuple of (data, status_code). It's non-trivial for flask-accepts to take only the object and determine the status code from it in anything but simplistic situations. A complete endpoint will have a lot of possible of different HTTP status codes. I usually have endpoints that return 200, 201, 204, 400, 401, 404, 422 and 500 status codes, so this seems like a strong requirement in a complete app.
2. Specify schemas for each status code of interest. Store the schemas in a dict of status_code -> schema. Cases where the schema is not specified can easily be detected using the corresponding KeyError.
3. Allow responds to be specified multiple times, once for each HTTP status code.

This feels like quite a large refactoring though, and is definitely a breaking change. I guess this should be discussed with @apryor6 :)

[volfpeter]

My two cents:
I just don't think forcing return values to be tuples is a good idea. As you said it's a huge breaking change and adds (in most cases unnecessary) complexity to routing for developers. To quote another somewhat related principle from the zen of Python, special cases are not special enough to break the rules.

If support for multiple status codes is added, it must be added in a way that i) doesn't break existing code, ii) is straightforward to use (no tuples, they lack semantics and could actually be data). For example there could be a Response class with mandatory data and code properties, plus maybe an optional headers property, as described in the issue. If the route returns a Response, use the schema corresponding to code if such a schema was registered on the route with responds, otherwise raise an exception. If the route returns anything else and only one responds decorator was applied on the route, use that schema, otherwise raise an exception.

[odesenfans]

To quote another somewhat related principle from the zen of Python, special cases are not special enough to break the rules.

I would say that this is not a special case, but rather an oversimplification from flask-accepts. Even a PUT must send different codes depending on whether the object was created or updated. HTTP codes need to be understood and controlled by developers.

I was proposing a tuple since it's the standard way of returning responses in Flask. I see this as more of a fix of a problem with flask-accepts rather than a new feature. Since we're not at a stable version number I would say the breaking change is acceptable and would make the library more usable in the long run.

[volfpeter]

There is no standard for building REST APIs, it's an API implementation style that uses HTTP methods to communicate semantics. Everything you put on top of this (like PUT must respond with different status codes whether an object was created or updated) is up to you. I actually prefer a simple API style like this (notice the author), but this is also just a preference.

I would argue that in Flask make_response is the standard, but returning simply the result (or maybe an extra status code) is enough in most cases so the devs were kind/smart and added it as a convenience feature for users. You're also making the assumption that flask-accepts is (or will be) following semantic versioning, which may not be true.

Anyway, we've moved to subjective topics, so I'm closing this on my part. Actually the only reason I commented was because not long ago another breaking, not perfectly implemented PR ( #80 ) was accepted here that had to be later reverted, and I would like a solution that doesn't cause trouble for the existing users of this excellent project.

[apryor6]

Hi! I agree with the premise that we don’t want to break backwards compatibility. Beyond that, a proposal that provides something like a dict mapping of status codes to associated schemas would seem solid. The decorator code can check for this argument or the type of the argument and react accordingly. This seems straightforward and prevents any need for stacking decorators. To extend an existing responds decorator, simply convert the single params to a dict of status_code:schema. I’d happily review a tested PR and cut a release, but candidly the likelihood that I can find time to address this any time soon is very small.

…

-AJ

Sent from my iPhone

On Oct 27, 2020, at 6:55 AM, Peter Volf ***@***.***> wrote:  There is no standard for building REST APIs, it's an API implementation style that uses HTTP methods to communicate semantics. Everything you put on top of this (like PUT must respond with different status codes whether an object was created or updated) is up to you. I actually prefer a simple API style like this (notice the author), but this is also just a preference. I would argue that in Flask make_response is the standard, but returning simply the result (or maybe an extra status code) is enough in most cases so the devs were kind/smart and added it as a convenience feature for users. You're also making the assumption that flask-accepts is (or will be) following semantic versioning, which may not be true. Anyway, we've moved to subjective topics, so I'm closing this on my part. Actually the only reason I commented was because not long ago another breaking, not perfectly implemented PR ( #80 ) was accepted here that had to be later reverted, and I would like a solution that doesn't cause trouble for the existing users of this excellent project. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[circulon]

@odesenfans
PR #123 Addresses your points 1 & 2.

I handled Point 3 slightly differently in that instead of multiple @responds statements you can provide a dict of status codes and their associated Schemas to the alt_schemas parameter instead .

A basic example

class LoginSchema(Schema):
    username = fields.String()
    password = fields.String()

class TokenSchema(Schema):
    access_token = fields.String()
    id_token = fields.String()
    refresh_token = fields.String()

class ErrorSchema(Schema):
    error_code = fields.Integer()
    errors = fields.List()
     
@api.route("/restx/update_user")
class LoginResource(Resource):
    alt_schemas = {
        401: ErrorSchema,
    }
    @accepts(schema=LoginSchema, api=api)
    @responds(schema=TokenSchema, alt_schemas=alt_schemas, api=api)
    def post(self):
        payload = request.parsed_obj
        username = payload.username
        password = payload.password

        tokens = self.attempt_login(username, password)
        if tokens is None:
            return {"error_code": 8001, "errors": ["invalid username or password"]}, 401
        
        return tokens