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

Functions clean_operations and clean_parameters refactored #463 #466

Closed
wants to merge 1 commit into from

Conversation

Pentusha
Copy link

@Pentusha Pentusha commented Jun 26, 2019

@lafrech
Copy link
Member

lafrech commented Jun 26, 2019

Hi @Pentusha. Thank you for your contribution.

I'll be AFK until next week so I won't be able to look at it until then.

I agree that having those method in the module is not ideal. We could provide them as APISpec class method or in a separate class like you did.

Before we do anything, could you please elaborate on your use case so that we have a better understanding of what is blocking?

I'd rather not add API surface (make more stuff public) if not needed, as it limits our ability to refactor.

@Pentusha
Copy link
Author

In my projects I'm using aiohttp + apispec + marshmallow.
I got custom plugin for apispec, where I implement some OAS3 features like discriminators. I also drop support of OAS2 and old python versions and I do not have a goal to maintain compatibility with older versions.
I implement several decorators, and functions, that gives me a declarative way to describe my API. When I call it, it puts component to my specification components/(schemas|responses|parameters), and my handler decorator just build references to components. So your clean methods and problems you solved just irrelevant in my case. Also I need ability to override reference builder.

class CustomAPISpec(APISpec):
    ....
class API:
    def __init__(...):
        self.spec = CustomAPISpec()
    # ...
    def parameter(self, field: fields.Field, name: str, component_name: str,
                  location: str, required: bool = False):

        field_schema = custom_marshmallow_plugin.openapi.fields2jsonschema(
            fields={component_name: field},
        )

        assert field.metadata.get('description')
        self.spec.components.parameter(
            name=name,
            component_id=component_name,
            description=field.metadata['description'],
            location=location,
            schema=field_schema['properties'][component_name],
            required=required,
        )
        return ParamWrapper(
            field=field,
            name=name,
            location=location,
            component_name=component_name,
            required=required,
            description=field.metadata['description'],
        )

api = API()
offset_param = api.parameter(
    field=fields.Int(
        description='Number of items to skip before returning the results',
        default=0,
        validate=[
            validate.Range(min=0),
        ],
    ),
    name='offset',
    component_name='offset_param',
    location='query'
)

@api.handler(
    summary='Stories list',
    response=StoriesListResponseSchema,
    params=[limit_param, offset_param],
    tags=['stories'],
)
async def stories_list(request):
    # ...

@jeffsawatzky
Copy link

Would any of this work help me with my issue #479?

@Pentusha
Copy link
Author

Would any of this work help me with my issue #479?

Yes, it may help you with your issue and your issue may help me. My handler decorator generate references like you doing it. Anyway your code should be valid.

@lafrech
Copy link
Member

lafrech commented Aug 31, 2019

Sorry for the delay.

From the example, it is not obvious to me what is blocking the use case in current implementation, but I agree on the principle that using functions from module scope doesn't help subclassing.

What do you guys think about making those cleanup functions APISpec methods?

Rathern than a separate class, we could use a mixin, like we just did with FieldConverterMixin and like I suggested in #488 for OpenAPIConverter, but I lean towards just making them APISpec methods.

Here's a proposal: #489.

@lafrech
Copy link
Member

lafrech commented Sep 1, 2019

Achieved differently in #489.

@lafrech lafrech closed this Sep 1, 2019
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

Successfully merging this pull request may close these issues.

None yet

3 participants