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

Make it possible to register the same model definition multiple times in doc.definitions #103

Closed
volfpeter opened this issue Jun 5, 2019 · 9 comments

Comments

@volfpeter
Copy link
Contributor

Hi,

We plan to document our API with sanic-openapi, I'm currently implementing some tooling for it. We tried to reuse some consumed/produced model definitions in the doc of multiple API endpoints, but it turns out reusing the same class wrapped in different doc.Object instances is not supported by the framework, even if object_name is different in every case.

Would it be possible to register Object instances in definitions with self.object_name instead of self.cls? I've tested it locally and it seems to work.

Here is my workaround code:

class Object(doc.Object):

    def __init__(self, cls, *args, object_name=None, **kwargs):
        super().__init__(cls, *args, object_name=object_name, **kwargs)
        
        doc.definitions[self.object_name] = doc.definitions[self.cls]
        del doc.definitions[self.cls]

PS.: Thanks for your efforts!

@chenjr0719
Copy link
Member

Hi there,

About reuse doc.Object, I have been used it in my company product like this:

from sanic import Sanic
from sanic.response import json

from sanic_openapi import doc, swagger_blueprint

app = Sanic(strict_slashes=True)
app.blueprint(swagger_blueprint)


class PaginationObject:
    start = doc.Integer()
    limit = doc.Integer()


class ListResourceAProducer:
    pagination = doc.Object(PaginationObject)
    foo = doc.String()


class ListResourceBProducer:
    pagination = doc.Object(PaginationObject)
    bar = doc.String()


@app.get("/resource_a")
@doc.summary("List all resource a.")
@doc.produces(ListResourceAProducer)
async def list_resource_a(request):
    return json({"pagination": {"start": 0, "limit": 10}, "foo": "foo"})


@app.get("/resource_b")
@doc.summary("List all resource b.")
@doc.produces(ListResourceBProducer)
async def list_resource_b(request):
    return json({"pagination": {"start": 0, "limit": 10}, "bar": "bar"})


if __name__ == "__main__":
    app.run()

The swagger looks like:
image
image

Does this cover your need?

@volfpeter
Copy link
Contributor Author

Thanks for the quick response. My use-case is a bit more complicated, but it can be condensed into something like this code:

class ConsumedModel:
    foo = str
    bar = str

@app.post("/foo")
@doc.consumes(doc.Object(ConsumedModel, object_name="FooConsumes"), content_type="application/json", location="body")
def foo(request: Request):
    return "foo"

@app.post("/bar")
@doc.consumes(doc.Object(ConsumedModel, object_name="BarConsumes"), content_type="application/json", location="body")
def bar(request: Request):
    return "bar"

As you will see, in this case only the /foo endpoint gets documented and only FooConsumes will be in the Models section.

Back to the proposed solution: I can see that it might cause problems for users who declare different models with the same name in multiple modules for example, but using the fully qualified class name instead of cls.__name__ as the default value of self.object_name and then using self.object_name as the key in definitions fixes this as well.

@chenjr0719
Copy link
Member

Got your point, thanks for your case sharing. I also totally agree with your solution. Would you wanna submit a PR for it?

@volfpeter
Copy link
Contributor Author

Sure, i can submit a PR. According to the readme, Python 3.5 must be supported (no f-strings). Will it still be the case in the next release?

@chenjr0719
Copy link
Member

In my personal opinion, drop python 3.5 support is totally acceptable since sanic also drop it. But we don't have any plan now.

@volfpeter
Copy link
Contributor Author

Please have a look the the pull request (#104). Thanks!

@ahopkins
Copy link
Member

ahopkins commented Jun 6, 2019

Just a bit of clarification about why Sanic decided to drop Python 3.5 beginning with the 19.6 release...

The decision was not because we had any syntax we particularly had in mind. In fact, there are still downloads of Sanic using Python 3.5. But, since we have an long-term support release (v 18.12), we decided to extend that support thru the sunseting of Python 3.5 in September 2020.

Therefore, Sanic will continue to work on Python 3.5 as long as that release of Python is officially supported, albeit on the 18.12LTS.

As for the decision why 19.6 is dropping it, the issue mainly has to do with the reliance upon third-party libraries needed before ASGI could be adopted.

Some background


With that said, I would encourage this project to continue supporting 3.5 until the 18.12LTS is no longer supported UNLESS there is a specific need.

@volfpeter
Copy link
Contributor Author

Thanks for this explanation. The PR is Python 3.5 compatible.

@chenjr0719
Copy link
Member

@ahopkins Sorry for the answer that not clear enough. But it also reminds me to put something in my todos:

  1. Contributing guide: I want to put your response into CONTRIBUTING.md to clarify what position of this project about supporting which version of Python.
  2. Tox: Current tox.ini in this project only using Python 3.5 and 3.6 with the latest version of Sanic on PyPI to run tests(at this time, it is 19.03.1). I want to expand it to using Python 3.5, 3.6, 3.7 with Sanic 18.12LTS, and all releases after LTS.
  3. Pull Request template: I want to put a checklist including read the contributing guide, run tox before submit PR and etc in the PR template to remind contributors who want to submit a PR to this project,

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