marshmallow-code / flask-smorest

DB agnostic framework to build auto-documented REST APIs with Flask and marshmallow
https://flask-smorest.readthedocs.io
MIT License
660 stars 73 forks source link

How to structure a real world application #38

Open svidela opened 5 years ago

svidela commented 5 years ago

Hi,

I believe that due to recent updates in both apispec and marshmallow my application started to throw warnings and exceptions (e.g., marshmallow.exceptions.RegistryError, apispec.exceptions.DuplicateComponentNameError) that were not happening a few weeks ago.

I couldn't came up with a minimal example to illustrate this yet. But I wonder if you could provide an example (beyond the basic "Petstore example" from the docs) of how to structure a more realistic application (like using the application factory pattern, splitting schemas and views in different modules, etc).

Regards,

svidela commented 5 years ago

Hi again,

I wanted to share the 2 things I had to do in order to adapt my application to be compatible with flask-rest-api 0.13 ( actually, apispec>1.0.0b6, marshmallow>3.0.0rc3).

marshmallow.exceptions.RegistryError: Class with name 'PetSchema' was not found. You may need to import the class.

when using the Api.definition decorator with a schema having a nested field declared with a class name string like in two-way nesting in marshmallow schemas .

You can solve this either not using the Api.definition or making sure to import all your schemas before calling init_app.

apispec.exceptions.DuplicateComponentNameError: Another schema with name "Pet" is already registered.

when using the Api.definition decorator, or

UserWarning: Multiple schemas resolved to the name Person. The name has been modified. Either manually add each of the schemas with a different name or provide a custom schema_name_resolver. UserWarning,

when the Api.definition decorator is not used at all but there are nested fields declared with modifiers (like only=('id', )).

In the first case (using the Api.definition decorator), the issue is that apispec is also registering schemas in the spec's components using its default schema_name_resolver.

In the second case (not using the Api.definition and having nested fields with modifiers), apispec tries to register the same schema name for each modifiers combination used and shows the warning.

The workaround I found for this is to don't let apispec to register nested schemas automatically and use the Api.definition decorator. For this I call init_app passing a custom marshmallow plugin:

from flask import Flask

from flask_rest_api import Api
from apispec.ext.marshmallow import MarshmallowPlugin

api = Api()

def create_app():
    app = Flask(__name__)

    ma_plugin = MarshmallowPlugin(schema_name_resolver=lambda _: False)
    api.init_app(app, spec_kwargs=dict(marshmallow_plugin=ma_plugin))

    return app

For a minimal example reproducing these two issues you may want to checkout flask-rest-api-example.

Regards,

lafrech commented 5 years ago

Thanks @svidela for the feedback. These are in fact apispec issues. The auto-registration feature is brand new. It is great but it has a few limitations we found acceptable.

As you understood already, you can opt-out by passing a no-op schema_name_resolver. But it would be a pity.

Make sure to import all my schemas before calling init_app

Out of my head, the requirement is that such nested schemas are registered before their parent. This is an apispec limitation related to the auto-registration feature. I don't think calling init_app before of after registration matters. It is just the relative order of registration that matters.

apispec.exceptions.DuplicateComponentNameError: Another schema with name "Pet" is already registered.

when using the Api.definition decorator

This is normal if you're calling Api.definition several times with the same name. If you want to register multiple modified versions of a schema, use Api.definition with different names.

UserWarning: Multiple schemas resolved to the name Person. The name has been modified. Either manually add each of the schemas with a different name or provide a custom schema_name_resolver.
UserWarning,

when the Api.definition decorator is not used at all but there are nested fields declared with modifiers (like only=('id', )).

This is just a warning telling you that the resolver had to make up different names, so he defaulted to Pet, Pet1, etc. To avoid this, either pass a custom resolver that will generate specific names based on the modifiers or even on a Schema <-> name mapping (which would be pretty much equivalent to manual registration), or manually register modified schemas with custom names.

Before the auto-registration feature, I tried to register the nested schema manually to get a better documentation. It was verbose, a bit painful and incomplete. Upgrading my app to recent apispec versions allowed me to remove a lot of manual registration and get a better result. My point is it would be too bad to opt-out of the feature. It is worth trying to cope with its limitation.

Of course, if some of those limitations can be gracefully addressed, we'd be happy to do that.

About the real world application example, I don't know when I'll get the time to do that. If I do it, the example may stay away from specific cases such as two-way nesting and all, so I don't think it would help with such problems. But I agree it would be nice to have a more complete example.

svidela commented 5 years ago

Hi @lafrech!

Thanks for your answer!

This is normal if you're calling Api.definition several times with the same name. If you want to register multiple modified versions of a schema, use Api.definition with different names.

Actually, I was not calling Api.definition several times with the same name. I call @api.definition('Pet') only once but (as far as I understand) apispec is also auto-registering a schema with name 'Pet' and that's why the exception is raised.

About using the auto-registration feature, I'm using the following approach to cope with schemas having nested fields using modifiers.

class PersonSchema(Schema):
    id = fields.Int(dump_only=True)
    name = fields.String()

    pets = fields.Nested('PetSchema', many=True)

class PersonNestedSchema(PersonSchema):
    class Meta:
        fields = ('id', 'name')

class PetSchema(Schema):
    id = fields.Int(dump_only=True)
    name = fields.String()

    owner = fields.Nested(PersonSchema)

class CarSchema(Schema):
    id = fields.Int(dump_only=True)
    name = fields.String()

    # This shows a UserWarning from `apispec` due to the name 'Person' being registered twice
    # owner = fields.Nested(PersonSchema, only=('id', 'name'))

    # Using a different schema class solves the issue
    owner = fields.Nested(PersonNestedSchema)

Cheers!

lafrech commented 5 years ago

Actually, I was not calling Api.definition several times with the same name. I call @api.definition('Pet') only once but (as far as I understand) apispec is also auto-registering a schema with name 'Pet' and that's why the exception is raised.

Yeah, exactly. If you call definition first, the auto-registration feature will make up a name and issue a warning. If you call definition last, after the schema is auto-registered, you get an exception.

I think your approach is the right one.

(When pasting code, you can specify the language (```py) to get syntax highlighting.)

revmischa commented 5 years ago

Thanks, I'm also experiencing this exact issue - reported at https://github.com/Nobatek/flask-rest-api/issues/57

lafrech commented 5 years ago

We just realized that thanks to Schema auto-registration in apispec, the schema (formerly definition) decorator has become useless. In the general case, you can rely on auto-registration and don't even need to use it.

The good thing about that is that you don't need to import the Api instance in the views. Better, you don't even need to create the instance at import time, you can create it at app init. I think this solves https://github.com/Nobatek/flask-rest-api/issues/27 because each app gets a different Api instance.

from flask import Flask
from flask_rest_api import Api

from modules.users import blp as users_blp
from modules.jobs import blp as jobs_blp

def create_app():
    app = Flask(__name__)
    api = Api(app)
    # In a real-life app, I'd add a register_blueprints method in modules.__init__.py
    api.register_blueprint(users_blp)
    api.register_blueprint(jobs_blp)
    return app

I'm thinking of removing the schema decorator. The auto-registration mechanism does things pretty well by itself. Exotic cases should be addressed by either setting a custom schema_name_resolver to MarshmallowPlugin

from flask_rest_api import Api as ApiOrig
from flask_rest_api.spec import MarshmallowPlugin

def schema_name_resolver(schema):
    # whatever
    ...
    return name

class Api(ApiOrig):

    def __init__(self, app=None, *, spec_kwargs=None):
        spec_kwargs = spec_kwargs or {}
        spec_kwargs['marshmallow_plugin'] = MarshmallowPlugin(
            schema_name_resolver=schema_name_resolver)

        super().__init__(app, spec_kwargs=spec_kwargs)

or by using the schema method from the spec, but at app init time

api.spec.components.schema('CustomPetSchemaName', schema=PetSchema)

Using a decorator to register schemas is something I added in flask-rest-api. The schema decorator is a wrapper around apispec's schema method, which does not work as a decorator.

It looked neat, but perhaps it was not such a good idea. People have been confused and have reported circular import issues. Removing all this would make the structure clearer.

@svidela, would you like to try that and tell me what you think?

svidela commented 5 years ago

Hi @lafrech!

I agree that relying on apispec auto-registration is the way to go. I just updated my minimal example at flask-rest-api-example.

Cheers!

revmischa commented 5 years ago

Hey that's cool @svidela. Take a look at our flask-rest-api serverless starter kit https://github.com/jetbridge/sls-flask

lafrech commented 5 years ago

Great @svidela.

Thank you for publishing your example.

I'll remove the schema decorator (#75).

May we close #27, then? A single Api instance still can't extend two flask apps, but there is no point doing it anymore if Api can be instantiated at app init and each app has its instance.

lafrech commented 5 years ago

I've been working on a simple real-life example and I came up with a minimal application using sqlalchemy.

https://github.com/lafrech/flask-smorest-sqlalchemy-example

Not done (are such things ever done ?) but feedback welcome already.

lafrech commented 3 years ago

I just updated https://github.com/lafrech/flask-smorest-sqlalchemy-example.

The repo still misses CI and dependabot config.