TestBoxLab / chalice-spec

Chalice x APISpec x Pydantic plug-ins
MIT License
17 stars 7 forks source link

Add more options for documentation #11

Closed hamishfagg closed 1 year ago

hamishfagg commented 1 year ago

This PR adds options to populate swagger docs more completely through tags, summaries, descriptions, and path parameters.

I've gone with the philosophy that a package such as chalice-spec should try to do as much as possible automatically, because no one likes writing docs πŸ˜„. So by default they will populate all of the below automatically - but the automatic behaviour can be overwritten by setting any of these options on an Operation object.

Path parameters

These are inferred from the path itself. Any identifiers inside curly braces in a path is added as a string path parameter for that path (e.g. for the path /users/{id}/friends/{f_id} will add the path parameters id and f_id to the spec).

Tags

A tag is added to each operation which is the first section of the path. /users, /users/{id} and /users/{id}/friends/{f_id} will all be tagged with /users. This just serves to group endpoints somewhat in the docs.

Summary and Description

These are simple strings that if not defined in an Operation will be inferred from the route function's docstring. The first line of the docstring will be used as the summary, and the rest is used as the description.

I haven't updated chalice-spec docs or anything, I'm interested to hear what you think of the above first.

hamishfagg commented 1 year ago

Here's an example of the PR working. For the function def below, the /users tag, description, summary, and path params are all being generated automatically.

@app.route('/users/{id}', methods=['GET'], docs=Docs(response=UserSchema))
def get_user(id):
    """
    Retrieve a user object.
    User's can't retrieve other users using this endpoint - only themselves.
    """

image

hamishfagg commented 1 year ago

Any thoughts on this? We have been using this for a couple of weeks and it's working well.

jakemwood commented 1 year ago

Hey @hamishfagg! Apologies for the delay, I was OOO for 2 weeks on vacation and haven't had a chance to look at this in depth yet since I've been back. I'll be doing a more in-depth code review soon! 😁

hamishfagg commented 1 year ago

I've added some docs (and fixed up the logic a bit), tell me what you think :) reformatted too. No worries about the delay

hamishfagg commented 1 year ago

i guess the tests need updating too πŸ˜› If you can do that great, otherwise itll have to be after the weekend for me

EricBujequeEmburse commented 1 year ago

Hey guys, I am very interested in this feature, you did excellent work. When do you think it will be released? I could help to finish it from the next week if needed.

EricBujequeEmburse commented 1 year ago

@hamishfagg I updated the current tests but I have no right to push a commit into your fork. Can you give me the proper right to push the commit?

Edit: I also added tests for this specific feature. I will push the code when I have writing rights.

hamishfagg commented 1 year ago

Done =)

jakemwood commented 1 year ago

Hey @hamishfagg! We made a pretty hefty API change out from under you (see #12 for details). I've gone ahead and tried to fix up the merge conflict as best I can. Once the formatting and tests are passing, we'll get it merged.

After it's merged, just as an FYI, I plan on introducing configuration options so people can turn on and off these defaults as they please. Some users may not want the tags, parameters, etc., activated by default. This might introduce a breaking change for you, so just make sure you have chalice-spec pinned to a specific version until we reach a 1.0 release. πŸ™‚

EricBujequeEmburse commented 1 year ago

Hey @jakemwood, @hamishfagg, I just pushed a new code that keeps the new feature functionality after the refactor and all the tests are passing. We need to merge and resolve the README.md conflicts to close this PR.

hamishfagg commented 1 year ago

That should do it? :)

jakemwood commented 1 year ago

Released in 0.5.0 πŸŽ‰