Giter Club home page Giter Club logo

Comments (17)

asyncee avatar asyncee commented on August 19, 2024 1

Thanks! I will give it a try in a few days and will report back about results.

from flask-smorest.

asyncee avatar asyncee commented on August 19, 2024 1

I wrote some test code with subclassing and overriding and it works.

Main problem of such approach i see is that too much logic got to be overriden, so if base library will change some decorator-specific logic, then things may break and my custom overriden decorators will differ from library provided ones.

What i'm talking about is, for example, response decorator:

# this logic is overriden in decorator

      if disable_etag:
            disable_etag_for_request()

            # Check etag precondition
            check_precondition()

            # Store etag_schema in AppContext
            set_etag_schema(etag_schema)

            # Execute decorated function
            result = func(*args, **kwargs)

            # Verify that check_etag was called in resource code if needed
            verify_check_etag()

Also error handler decorator — it has payload and headers mangling code.

So any of these decorators has 2 responsibilities:

  1. build response payload that will be jsonified later
  2. build json response

The problem is in second — once response is built it is hard to manipulate with, so approach to decorate provided decorators is not working, and one must override it as a whole.

It would be great if you can split these decorators into two parts:

  1. function or class that prepares response body (python datastructure)
  2. function that builds flask response (jsonify, update headers, etc)

So example may look like this:

"""Exception handler"""

from flask import jsonify, current_app
from werkzeug.exceptions import HTTPException, InternalServerError


def prepare_error(error) -> dict:
    payload = {
        'status': str(error),
    }

    # Get additional info passed as kwargs when calling abort
    # data may not exist if HTTPException was raised not using webargs abort
    # or if not kwargs were passed (https://github.com/sloria/webargs/pull/184)
    data = getattr(error, 'data', None)
    if data:
        # If we passed a custom message
        if 'message' in data:
            payload['message'] = data['message']
        # If we passed "errors"
        if 'errors' in data:
            payload['errors'] = data['errors']
        # If webargs added validation errors as "messages"
        # (you should use 'errors' as it is more explicit)
        elif 'messages' in data:
            payload['errors'] = data['messages']
        # If we passed additional headers
        if 'headers' in data:
            headers = data['headers']

    return payload


def handle_http_exception(error, prepare_func):
    log_info = True

    if not isinstance(error, HTTPException):
        error = InternalServerError()
        # Flask logs uncaught exceptions as ERROR already, no need to log here
        log_info = False

    headers = {}

    payload = prepare_func(error)

    # Log error as INFO, including payload content
    if log_info:
        log_string_content = [str(error.code), ]
        for key in ('message', 'errors'):
            if key in payload:
                log_string_content.append(str(payload[key]))
        current_app.logger.info(' '.join(log_string_content))

    return jsonify(payload), error.code, headers


# This is part of App.init_app method
    ...py

    for code in default_exceptions:
        # App may define prepare_error_response function as prepare_func above.
        # Same would go for prepare_success_response
        error_handler = partial(handle_http_exception, prepare_func=self.prepare_error_response)
        app.register_error_handler(code, handle_http_exception)


     @staticmethod
     def prepare_error_response():
           return error_handler.prepare_error

So basically payload generation is split from flask response generation, so to implement custom logic, one just need to override Api.prepare_error_response and Api.prepare_success_response methods.

from flask-smorest.

lafrech avatar lafrech commented on August 19, 2024 1

This makes total sense.

I'll be looking into this. (Can't promise when.)

from flask-smorest.

asyncee avatar asyncee commented on August 19, 2024 1

Great to hear! I think it is okay to use current code version and to not to push deeper (yet).

I think issue can be closed after release, i will integrate the code into existing project and test it, it may take some time.

Anyway for me this project is the best (compared to more mature projects), because it uses marshmallow and saves time on writing documentation.

Thanks again!

from flask-smorest.

lafrech avatar lafrech commented on August 19, 2024 1

0.9.0 released!

from flask-smorest.

lafrech avatar lafrech commented on August 19, 2024

Hi @asyncee. Thanks for the positive feedback.

flask-rest-api was built over a few opiniated choices, to fit our own needs, and this is not the format we're using, so it is not supported out-of-the-box.

However, after a very quick look, I think there may be a way to do it easily by just

  • subclassing Blueprint to override response decorator, wraping its output content into your structure
  • wrapping or overriding handle_http_exception to do the same with error messages

You might have to also subclass Api to override init_app to register your handler in place of the one in flask-rest-api (I don't know what happens if multiple handlers are registered).

Please tell us how it goes if you give it a try. I could take the error handler registration out of init_app into a separated method if it helps subclassing.

from flask-smorest.

asyncee avatar asyncee commented on August 19, 2024

That is just great, thank you very much.

from flask-smorest.

lafrech avatar lafrech commented on August 19, 2024

Did you also manage to get the docs to correctly expose the wrapping structure?

from flask-smorest.

asyncee avatar asyncee commented on August 19, 2024

Nope, i missed this part, sorry. I think that currently available doc-generating machinery could be in use without changes if prepare_error_response and prepare_success_response will define it's marshmallow schema for wrapping response.

from flask-smorest.

lafrech avatar lafrech commented on August 19, 2024

@asyncee, I gave it a try. I took a different approach. I'm not 100% happy with it yet. It looks a bit awkward, but fonctionally, it seems to provide flexibility.

https://github.com/Nobatek/flask-rest-api/tree/dev_rework_response_and_error

Please tell me what you think.

Note we still lack the possibility to define error response schema. This is a TODO, independent from this feature.

from flask-smorest.

asyncee avatar asyncee commented on August 19, 2024

I have quickly read the code and think it will make it. I will check it out in more detail in a day or two and post back.

Thanks!

from flask-smorest.

asyncee avatar asyncee commented on August 19, 2024

It works for me.

This is my code (i removed all comments to make it cleaner):

class ReworkApi(Api):

    @staticmethod
    def _prepare_error_reponse_content(error):

        headers = {}
        payload = {'status': str(error), 'type': 'error'}

        # Note: added <or getattr(error, 'description')> to get description from HTTPException
        data = getattr(error, 'data', None) or getattr(error, 'description')
        if data:
            if 'message' in data:
                payload['message'] = data['message']
            if 'errors' in data:
                payload['errors'] = data['errors']
            elif 'messages' in data:
                payload['errors'] = data['messages']
            if 'headers' in data:
                headers = data['headers']

        return payload, headers


class ReworkBlueprint(Blueprint):
    @staticmethod
    def _make_doc_response_schema(schema):
        if not schema:
            return

        class WrappingSchema(ma.Schema):
            type = ma.fields.String(required=True)
            data = ma.fields.Nested(schema)

        return WrappingSchema()

    @staticmethod
    def _make_response(data):
        data = {"type": "success", "data": data}
        return jsonify(data)

It gives me such documentation examples:

2018-09-26_14-55-14

image

Thank you for your efforts!

Although it looks like there are hidden entity, ResponseHandler or something like this. New Api and Blueprint mixins in fact solve same problem — prepare response and format payload, so it looks like this functionality may be extracted to specific class, so if one want to tune responses it could be done from one single place in the system.

Anyway, the code is usable for my specific task.

from flask-smorest.

lafrech avatar lafrech commented on August 19, 2024

Regarding the response schema, in you case, I think I would have sloppily wrapped all schemas and passed wrapped schemas to existing response decorator. This can be done with a factory that takes the original schema and returns the wrapped one.

But yes, with the rework, it is more elegantly solved at framework level.

Although it looks like there are hidden entity, ResponseHandler or something like this. New Api and Blueprint mixins in fact solve same problem — prepare response and format payload, so it looks like this functionality may be extracted to specific class, so if one want to tune responses it could be done from one single place in the system.

Perhaps. I get your point but I'm not sure it would be practically doable. But I'm open to refactor suggestions.

I've pushed in the same branch a similar refactor of the pagination decorator. I'm happy with how it looks. It should be merge soon. I need to update the docs first. Time to provide insights if you get the time.

Thanks.

from flask-smorest.

lafrech avatar lafrech commented on August 19, 2024

I just pushed the whole rework into master branch. I shall release 0.9.0 soonish.

from flask-smorest.

asyncee avatar asyncee commented on August 19, 2024

Great to hear, thanks!

from flask-smorest.

azzamsa avatar azzamsa commented on August 19, 2024

Then how to envelope our response?

I don't find any clue in the docs: https://flask-smorest.readthedocs.io/en/latest/response.html

I tried, to extend my Api class with the code above, but didn't work:

"""Api extension initialization

Override base classes here to allow painless customization in the future.
"""
import marshmallow as ma
import flask_smorest as flasksm
from flask import jsonify


class Blueprint(flasksm.Blueprint):
    """Blueprint override"""

    @staticmethod
    def _make_doc_response_schema(schema):
        if not schema:
            return

        class WrappingSchema(ma.Schema):
            type = ma.fields.String(required=True)
            data = ma.fields.Nested(schema)

        return WrappingSchema()

    @staticmethod
    def _make_response(data):
        data = {"type": "success", "data": data}
        return jsonify(data)


class Api(flasksm.Api):
    """Api override"""

    def __init__(self, app=None, *, spec_kwargs=None):
        super().__init__(app, spec_kwargs=spec_kwargs)

    @staticmethod
    def _prepare_error_reponse_content(error):

        headers = {}
        payload = {"status": str(error), "type": "error"}

        # Note: added <or getattr(error, 'description')> to get description from HTTPException
        data = getattr(error, "data", None) or getattr(error, "description")
        if data:
            if "message" in data:
                payload["message"] = data["message"]
            if "errors" in data:
                payload["errors"] = data["errors"]
            elif "messages" in data:
                payload["errors"] = data["messages"]
            if "headers" in data:
                headers = data["headers"]

        return payload, headers

        # Register custom Marshmallow fields in doc
        # self.register_field(CustomField, 'type', 'format')

        # Register custom Flask url parameter converters in doc
        # self.register_converter(CustomConverter, 'type', 'format')


class Schema(ma.Schema):
    """Schema override"""

My other attemp is using marsmallow @post_dump, but will litter the model in generated swagger doc.

thank you

from flask-smorest.

azzamsa avatar azzamsa commented on August 19, 2024

solution:

The correct way to extends _prepare_response_content is not under class Api(flasksm.Api): but class Blueprint(flasksm.Blueprint)

class Blueprint(flasksm.Blueprint):
    """Blueprint override"""

    @staticmethod
    def _prepare_response_content(data):
        if data is not None:
            return {"data": data}
        return None


class Api(flasksm.Api):
    """Api override"""

    def __init__(self, app=None, *, spec_kwargs=None):
        super().__init__(app, spec_kwargs=spec_kwargs)


class Schema(ma.Schema):
    """Schema override"""

from flask-smorest.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.