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

Add Components.security_scheme #375

Merged
merged 2 commits into from
Feb 3, 2019
Merged

Add Components.security_scheme #375

merged 2 commits into from
Feb 3, 2019

Conversation

@lafrech
Copy link
Member Author

lafrech commented Feb 1, 2019

I realize there is not much added value in this. It abstract the location of the component w.r.t OpenAPI version but the content differs in both versions and there is no need for a helper, so passing the dict manually at APISpec instantiation as done in test_core.py is not much worse:

https://github.com/marshmallow-code/apispec/blob/dev/tests/test_core.py#L51

BTW, if we merge this, it could be worth finding a better example in test_core.py or adding a note saying the Component.security_scheme method is the preferred way to register security schemes.

@lafrech
Copy link
Member Author

lafrech commented Feb 1, 2019

I find it weird to pass the dict as kwargs rather than as a positional argument. I did this to expose an interface similar as parameter or response.

Maybe we could change that in all component registration methods, I mean change

    def parameter(self, param_id, location, **kwargs):
    def response(self, ref_id, **kwargs):

into

    def parameter(self, param_id, location, parameter, **kwargs):
    def response(self, ref_id, response, **kwargs):

but after a quick glance I realized it has consequences on the plugin interface (helpers signature), and maybe more, so I left it as is for now.

Since there is no helper and therefore no kwargs to pass, we could use a positional argument here, but it would be less consistent. And we'd have to break that if someone ever comes up with a security scheme helper.

Copy link
Member

@sloria sloria left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm good with this as is.

Even though the use case is supported by passing kwargs to APISpec, this is consistent with the rest of the API.

@lafrech
Copy link
Member Author

lafrech commented Feb 1, 2019

docs/special_topics.rst needs to be updated.

@sloria sloria merged commit be45c95 into dev Feb 3, 2019
@sloria sloria deleted the add_security_scheme_component branch February 3, 2019 19:39
@lafrech lafrech changed the title Add component.security_scheme Add Components.security_scheme Nov 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants