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

[Suggestion] ResourceConfig refactor #73

Open
fenekku opened this issue Jul 27, 2020 · 2 comments
Open

[Suggestion] ResourceConfig refactor #73

fenekku opened this issue Jul 27, 2020 · 2 comments

Comments

@fenekku
Copy link
Contributor

fenekku commented Jul 27, 2020

I think ResourceConfig can be improved:
1- ResourceConfig is starting to accumulate a hodgepodge of fields, but these are not quite organized, making it hard to navigate the configuration (especially for developers making an API using this library) and extend it without being overwhelmed.
2- Some custom classes/objects are needed in the configuration which adds to the difficulty of navigating it and creating it. Some we have no choice (Any Serializer/Deserializer), but others we could potentially remove from the config (only for now).

Here is my suggestion for the refactor of ResourceConfig taken from https://gist.github.com/fenekku/69b49923816cd9ab160aa46f0a70f1e3#file-resources-py-L13-L49

class ResourceConfig:    
    # Suggested
    item_route = "/resources/<id>"  # or request_item_route . These 2 don't matter too much to me
    list_route = "/resources/"

    ## incoming
    request_url_args_parser = ArgsParser()  # Could be renamed ArgsDeserializer()
    # OR
    request_url_args_parser = {
        "create": ArgsParser()  # any of the resource method can be listed
    }
    
    request_body_deserializer = [JSONDeserializer(mimetype="application/json"),],
    # OR
    request_body_deserializer = {
        "create": [JSONDeserializer(mimetype="application/json"),]
    }

    # Wouldn't worry about implementing this one until we actually need it
    request_headers_deserializer = [HeadersDeserializer(),],
    # OR
    request_headers_deserializer = {
        "create": [HeadersDeserializer(),]
    }
    
    ## outgoing
    # Optional and again would wait until needed to implement
    response_headers_serializer = HeadersSerializer(override=False)  
    # OR
    response_headers_serializer = {
        'create': HeadersSerializer(override=False)  # Optional
    }
    
    response_body_serializer = [JSONSerializer(mimetype="application/json"),]
    # or
    response_body_serializer = {
        'create': [JSONSerializer(mimetype="application/json"),]
    }

Nice things:

  • The symmetry between request and response is evident.
  • Developers can cater to specific Resource methods
  • Only Serializers/Deserializers need to be provided: no other custom objects.

This issue combines: #39 and #40 (which were both started from comments I made).

@ppanero
Copy link
Member

ppanero commented Jul 28, 2020

Would there be a way to allow the usage of the same classes for all methods? e.g. not having to set create, search, update, etc. separately? Some sort of "generic" and then specific that overwrite it. WDYT?

@fenekku
Copy link
Contributor Author

fenekku commented Jul 28, 2020

In the above, same class for all methods is allowed or per-class is allowed, but you mean being able to specify a class for all methods AND then override it for some methods, right?

That would be nice! Do you have a configuration in mind? I am thinking something like:

request_body_deserializer = [
    GeneralJSONDeserializer(mimetype="application/json")
    ("create", [CreateSepcificJSONDeserializer(mimetype="application/json"))
]

or

request_body_deserializer = {
    "all": [GeneralJSONDeserializer(mimetype="application/json"),]
    "create": [CreateSpecificJSONDeserializer(mimetype="application/json"),  
    CreateSpecificXMLDeserializer(mimetype="application/xml"),]
}

On another topic, Iam thinking a serializer/deserializer pair could be combined since they are often just the same schema anyway. To be given some thought...

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

3 participants