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

Support for http.HTTPStatus #60

Closed
jan-golda opened this issue Apr 25, 2019 · 2 comments · Fixed by #64
Closed

Support for http.HTTPStatus #60

jan-golda opened this issue Apr 25, 2019 · 2 comments · Fixed by #64

Comments

@jan-golda
Copy link

When I started using this library I wrote, out of habit, something like this:

@bp.response(code=HTTPStatus.NO_CONTENT)

(In my opinion using htttp.HTTPStatus is a good think because it prevents Magic Number anti-pattern)
It looked fine, but after a while i realized that instead of resulting in:

"responses": {
    "204": {}
}

It is rendered as:

"responses": {
    "HTTPStatus.NO_CONTENT": {}
}

So my suggestion is to add int() to this line:
https://github.com/Nobatek/flask-rest-api/blob/92229aaf74819ba9a87ff98d7888ca25661d7e4e/flask_rest_api/response.py#L49

@lafrech
Copy link
Member

lafrech commented Apr 26, 2019

This is about to be fixed in apispec: marshmallow-code/apispec#436.

Once this PR is merged, I'll remove the cast to str in flask-rest-api and everything will be handled in apispec. I think.

@lafrech
Copy link
Member

lafrech commented Apr 30, 2019

apispec PR merged.

There is still a little issue in flask-rest-api. When merging manual and automatic documentation, the status code, which is the key in the responses dict, must be in the same format in both auto and manual doc. Currently, the code is always cast to string in auto doc, so the user is expected to use string codes in manual doc (@blp.doc(responses={"200": ...}). Removing the cast to string means the user must use the same format as the one used in auto doc (default 200 is int, otherwise, the type used by the user when specifying the response status code).

    # This won't work because doc uses a string and response uses an int
    @blp.doc(response={'201': {...}})
    @blp.response(code=201)
    def view_func():
        ...

    # This works because doc uses an int and response uses an int (default)
    @blp.doc(response={200: {...}})
    @blp.response()
    def view_func():
        ...

Removing the cast does not make the situation much worse, but it would break code for users who are passing additional response doc with @blp.doc as string already.

There is no ideal solution to this. I'd rather not do fancy stuff with info passed in @blp.doc as it is meant to be a last resort. I'd rather provide more structured options.

This could be done later but it would be nice to have the latter in the same minor release as the cast removal.

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 a pull request may close this issue.

2 participants