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

Thoughts on refactor #94

Open
fenekku opened this issue Mar 31, 2021 · 0 comments
Open

Thoughts on refactor #94

fenekku opened this issue Mar 31, 2021 · 0 comments

Comments

@fenekku
Copy link
Contributor

fenekku commented Mar 31, 2021

Overall the refactor is quite good. Here is my list of notes / potential areas for improvements / fixes. They are mostly from the perspective of a developer using the library:

  • documentation fixes:

    • provide imports for larger example
    • add def delete(self) method so that create_url_rules makes sense
    • from_conf typo with additional apostrophe
    • add example of how to pass different urls (configurable urls)
    • clarify view_args are route arguments here (may have implementation impact)
    • clarify response_handler decorator is just for many=True, otherwise should not be needed unless we go the route of allowing it to override response_handlers on a per method basis.
  • error_handlers and response_handlers should mirror each other:

    • response_handlers could also be defined in the Resource and overridden/merged by the ResourceConfig
    • Like response_handlers dict, the error_handlers dict should take instances as opposed to the result of a function call:
      • create_error_handler replaced by ErrorHandler (analoguous to ResponseHandler)
  • request_body_parser could be merged into request_parser

    @request_parser(
        {'q': ma.fields.String(required=True)},
        # Other locations include args, view_args, headers.
        location='args',
    )
    @response_handler(many=True)
    def search(self):
        # The validated data is available in the resource request context.
        if resource_requestctx.args['q']:
            # ...
        # From the view you can return an object which the response handler
        # will serialize.
        return [], 200

    # You can parse request body depending on the Content-Type header.
    @request_parser(
        {"application/json": RequestBodyParser(JSONDeserializer())}
        location='body'    
    )

This is really for developer convenience: less bits to import and simpler semantics. Behind the scene we do the work of wiring thins together nicely.

  • still need to use it a bit more in practice (especially from_conf) to see how it feels
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

1 participant