Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

init spectree with plugin instead of name, use body instead of json #53

Closed
kemingy opened this issue Jul 15, 2020 · 7 comments
Closed
Labels
RFC Request for Comments

Comments

@kemingy
Copy link
Member

kemingy commented Jul 15, 2020

To keep the interface clean and flexible, it's better to pass the plugin class instead of backend framework name strings. Also, the sync/async flag should be written to the plugin class as described in https://github.com/0b01001001/spectree/pull/46/files#diff-485ad20a22f45089777317b26137dc90R13-R15

Since the body can be serialized in multiple ways, like JSON, MessagePack, ProtoBuf, etc. Although JSON should be the most common way to do so, it's better to support other methods.

@kemingy kemingy added the RFC Request for Comments label Jul 15, 2020
@kemingy
Copy link
Member Author

kemingy commented Aug 22, 2020

Considering 99% use-cases are JSON API

That's True.

To support multiple serialization methods, it may require the user to define their own handlers. Currently, it works fine for most of the use cases.

The interface may look like:

handler = HTTP_body_handler(
    content_type='application/json',
    serialize=json.dumps,
    deserialize=json.loads
)

# global default handler
api = SpecTree(handler=handler)

# endpoint handler
@api.validate(handler=handler)

@yoursvivek
Copy link
Contributor

I had some thought about these issues, while making changes for #100. I'm jotting them down.

  1. body and payload are two well known identifiers for what's currently called json. Renaming will clean up the function signature of any naming convention bias towards one [de]serialization method. json commonly refers to json built-in module or one of many drop-in json module replacements.

  2. While implementing enable view function annotation based type detection #100, I did end up creating a variable named json in the current namespace which refers to validated pydantic model for http body. And therefore, if json module was imported, it will be masked inside view function.

  3. It can be implemented with breaking changes deferred to later release as follows:
    a. Let validate function accept another keyword (preferably keyword only?) argument called body.
    b. As the very beginning of validate, check presence of json and body. Allow only one of them to be present or give one precedence over the other and log a warning message. Issue a deprecation warning informing that json parameter will be dropped after a particular release.
    c. Thereafter, in all internal code for spectree use body instead of json

  4. Since there will be one breaking change in validate, it will be prudent to consider making all arguments of validate keyword once, this is make it easier to add features at a later date. This is important because validate function is playing multiple roles viz validating data models (akin to webargs) and collecting model types for registry into openapi spec schema components. In future with addition of other de/serialization implementations, other requirements might pop up.

  5. On using handlers, I think spectree should accept any subclass of a base handler object with trait like:

class BaseHandler:
   def serialize(self, **kwargs):
        ...
   def deserialize(self, **kwargs):
        ...
  1. Handling multiple content type should also be an option and de/serialize functions can absorb the complexity of de/serialization for different protocols. Even in the realm of json only apis, multiple content-types can be in use like in vendor ct. Github allows all of the following for it's json api implementation.:
application/json
application/vnd.github+json
application/vnd.github.v3+json
application/vnd.github.v3+json
application/vnd.github.v3.raw+json
application/vnd.github.v3.html+json
application/vnd.github.v3.text+json
application/vnd.github.v3.full+json
# etc ...

c.f.: https://docs.github.com/en/free-pro-team@latest/rest/overview/media-types

@kemingy
Copy link
Member Author

kemingy commented Jan 11, 2021

I think we can standardize all these keywords now.

  • json -> body

Singular or Plural?

  • header or headers
  • cookie or cookies
  • query or queries

For the handlers, users may have multiple kind of content types. For example, multipart/form-data, x-www-form-urlencoded, application/json. So it should be a <content-type: Handler> dictionary?

For the breaking changes, we can add use semantic versioning. Old keywords will be warned in v1 and will be dropped in v2.

@yoursvivek
Copy link
Contributor

All of them seem good to me.

  1. body
  2. Plurals.
  3. <content-type: Handler>
  4. Semvar for deprecating old keywords.

@alexted
Copy link

alexted commented Apr 22, 2021

  • headers
  • cookies
  • query (or mb query_params, query_args)

@ula
Copy link
Contributor

ula commented Oct 22, 2021

I understand that these changes require more thought and lots of breaking changes but is there a timeline for these changes? thanks

@kemingy
Copy link
Member Author

kemingy commented Oct 26, 2021

I understand that these changes require more thought and lots of breaking changes but is there a timeline for these changes? thanks

@ula Actually, there is no timeline for this. I'm too busy these days. BTW, I will try to avoid the breaking by making the default behavior acts as it is now. I don't have a very clean solution now. Feel free to comment here if you have any suggestions.

@kemingy kemingy changed the title [break changes]: init spectree with plugin instead of name, use body instead of json init spectree with plugin instead of name, use body instead of json Oct 26, 2021
@0b01001001 0b01001001 locked and limited conversation to collaborators Oct 26, 2021
@kemingy kemingy closed this as completed Oct 26, 2021

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
RFC Request for Comments
Projects
None yet
Development

No branches or pull requests

4 participants