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

No way to add description to path parameters #23

Closed
congenica-andrew opened this issue Nov 16, 2018 · 13 comments · Fixed by #78
Closed

No way to add description to path parameters #23

congenica-andrew opened this issue Nov 16, 2018 · 13 comments · Fixed by #78
Milestone

Comments

@congenica-andrew
Copy link

I can't see any way to add a description to a path parameter. Using the @blp.doc decorator as in:

    @blp.doc(parameters=[{"name": "thing_id", "description": "The ID of the thing"}])

just results in adding another parameter with a duplicate name and only a description. I have a local fix for this in FlaskPlugin where I just scan the existing parameters and update rather than append if I find one with the same name. I can get this into a PR unless you have another way you'd rather approach this?

@lafrech
Copy link
Member

lafrech commented Nov 16, 2018

This is similar to #19 as both examples and descriptions are treated similarly.

Easy way, add metadata to the field:

    name = fields.String(description='User name', example='Philip J. Fry')

If you need to specify those descriptions using @doc, you may face issues until #19 is solved. Only OpenAPi v3 is affected.

@lafrech
Copy link
Member

lafrech commented Nov 16, 2018

That's assuming you're using Schema as arguments. Using dict is not officially supported. It is not tested. I have no fundamental objection, but we never needed it so I never cared to support it. See #2.

@congenica-andrew
Copy link
Author

I did see your changes there, but it looks like it's fixing a slightly different issue. I'm concerned with path parameters here, not schema arguments. The problem is that currently FlaskPlugin.path_helper has no other source of data other than what it can work out from the path, so there's no way you can pass in a description.

@lafrech
Copy link
Member

lafrech commented Nov 16, 2018

Hmm... I see.

FlaskPlugin.path_helper plays last and indeed it appends to the list of parameters. Your fix looks like a sensible approach.

I wonder if we could provide a more declarative way of passing those metadata.

Out of my head:

Allow a specific syntax for rules to pass parameters description and examples. Parse this in Blueprint.route and only pass plain flask route to add_url_rule.

/pet/<int:pet_id{description:'ID of the Pet'}>/

This sounds much too hackish.

Or just use **options to pass this information. I'd still need to get my head around this and check the implications.

In both cases, this info could be added to the doc in _store_endpoint_docs.

FlaskPlugin.path_helper would still require the fix mentioned above so as not to append the parameter, but rather deepupdate, with the manual version overwriting the version from rule_to_params.

@congenica-andrew
Copy link
Author

Well, for what it's worth, here congenica-andrew@fe427b1 is what I have already.

@congenica-andrew
Copy link
Author

Personally I don't really like the look of trying to add extra stuff into the route definition. How about a new decorator, something like

@path_descriptions(thing_id="The ID of the thing")

I've not got my head enough around how the library works to work out where this would go though.

@lafrech
Copy link
Member

lafrech commented Nov 17, 2018

I applied a variation of your fix in master. Would you like to test before I release?

Keeping this open for now as the fix does not address the part about passing manual parameter docs without blp.doc.

@lafrech
Copy link
Member

lafrech commented Nov 17, 2018

The spec says:

A unique parameter is defined by a combination of a name and location.

So matching name only is not enough. I modified the code to match location and name.

@congenica-andrew
Copy link
Author

Sorry, I've not been able to look at this for a couple of days, but I've just tested 0.11.1 from PyPi and it works as expected. The only issue I had is that initially I didn't add the in key to my doc updates and this makes the app fail to start with a KeyErrror in path_helper, so it might be worth adding some extra checking here.

Thanks for the quick fix for this though 🥇

@lafrech
Copy link
Member

lafrech commented Nov 21, 2018

Good.

I think my rationale here was that:

  • It fails at init, not at runtime, so the error is caught be the developer.
  • Using @blp.doc is a last resort, so it will always be rough along the edges. It provides access to the internals. Let's not try to do too much checks in there, but rather, let's provide nice interfaces so that developers don't need it in the first place.

Ultimately, I'd rather have a nicer way of passing path params doc. I would have liked to avoid yet another decorator, but I'm afraid this is the least awkward solution identified for now.

@lafrech
Copy link
Member

lafrech commented Nov 28, 2018

We could extend the issue to other parameters (not only path parameters).

Adding extra doc to parameters using

@doc(parameters=...)

won't work because parameters is a list and can't be deepupdated.

Not a big deal because auto doc is quite complete already so this shouldn't happen. But it might be good to have that use case in mind when working on a new decorator.

@lafrech
Copy link
Member

lafrech commented Jun 11, 2019

Path parameters apply to the whole route, not a function (get, post,...).

Their documentation could (should?) be added through the route decorator rather than passed in each function. (I'm thinking of the MethodView use case in particular, where a single call to route covers multiple functions.)

Let's follow marshmallow-code/apispec#453 and see if we can rework the Flask plugin path_helper to do that.

@lafrech
Copy link
Member

lafrech commented Jun 18, 2019

@congenica-andrew if you're still interested by this, you may want to take a look at #78.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants