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

openapi: implement response descriptions #11

Closed
ahl opened this issue Jul 5, 2020 · 4 comments · Fixed by #50
Closed

openapi: implement response descriptions #11

ahl opened this issue Jul 5, 2020 · 4 comments · Fixed by #50
Assignees

Comments

@ahl
Copy link
Collaborator

ahl commented Jul 5, 2020

OpenAPI gives us several places where we can put relevant information related to a response:

    "/hardware/racks/{rack_id}": {
      "get": {
        "description": "**1** Retrieve information about a rack specified by ID.",
        "operationId": "api_hardware_racks_get_rack",
        "parameters": [
          { "...": "..."}
        ],
        "responses": {
          "200": {
            "description": "**2** Successfully retrieved information about the rack.",
            "content": {
              "application/json": {
                "schema": {
                  "description": "**3** Information that describes a rack",
                  "type": "object",
                  "properties": {
                    "id": {
                      "type": "string",
                      "format": "uuid"
                    }
                  },
                  "required": [
                    "id"
                  ]
                }
              }
            }
          }
        }
      }
    },

**1** is for the description of the OpenAPI "operation" i.e. the endpoint. Dropshot derives this field from the doc comment on the handler function
**2** is for the description of the OpenAPI "response". In this context, we mostly case about successful responses as error codes will be handled more-or-less generically (see #7). Dropshot doesn't have any content in this field today.
**3** is for the description of the OpenAPI "schema" i.e. the datatype that the response produces. Dropshot derives this today from the doc comment on the structure type.

Question 1: Do we need all three? 1 seems useful as it describes the purpose of the interface. 2 and 3 seem a bit redundant, but I'd argue that we may want both: the datatype may be used in several places and we may want to customize the response description. For example, POST, GET, and PUT (create, view, update) may return the same structure but we may want the description for 2 or 3 to differ. Alternatively, we may just want 3, leaving 2 implied.

Question 2.1: if we want to get rid of one of 2 or 3, which should we keep? Should the text continue to come from the doc comment on the type?

Question 2.2: if we want to keep all 3, where should the description for 2 come from? One could imagine asking the developer to specify this in the #[endpoint {...}] attribute, but this seems like it might be kind of a nuisance. Alternatively, we could use the HttpTypedResponse implementations to have an automatically generated, semi-tailored description. For example, HttpResponseCreated would say something like "creation successful"; HttpResponseDeleted, "deletion successful"; HttpResponseOkObjectList, "list of entities".

@steveklabnik
Copy link
Contributor

I am not an OpenAPI expert. I would hope that if they have three places, there are three different reasons for something being there.

On my reading, 1 & 2 seem useful and distinct, but 3 seems the least useful. It, as you say, feels redundant to me. I would prioritize 2 over 3, because it feels semantically good. That is, describing the response feels like the right thing, rather than saying "you can understand the response by looking at the description in its schema."

I don't know if that makes sense, but that's how I'm seeing it right now. No super strong feelings.

@davepacheco
Copy link
Collaborator

davepacheco commented Jul 7, 2020

Looking at "PATCH /projects/foo" might show sharper contrast:

  1. might be "Update a Project by specifying only fields that should be changed".
  2. might be "Returns the complete updated state of the Project."
  3. might be "Describes a Project" or even "Projects help users organize resources" (i.e., a description of what this API resource is for)

I guess they're not that different. I think we want (3) for operation-independent documentation for types, but as you alluded to, it might not be clear from (3) alone exactly what you're looking at: if I update an instance, am I getting the old value or the new value?

It'd be nice to get the documentation for (2) from the HttpTypedResponse docs, but I think we want to use that documentation for regular Rust API documentation for Dropshot consumers. That is, I'd expect HttpResponseCreated<T>(t: T) to say something like "This is a specific type to return from your API handler function to return an HTTP 201 Created response whose body is the result of serializing t." rather than "creation successful".

Where else could we put it?

  • make this a const in HttpTypedResponse? I think this is what you were suggesting above. This way, each specific implementation would specify help text for it ("creation successful"). Presumably this would get plumbed through ApiEndpointResponse.
  • an attribute on the HttpTypedResponse type? This is pretty similar but might be more ergonomic?
  • the "endpoint" macro as you suggested: seems fine, agree it's not awesome

That's in order of my preference but I don't have strong feelings about it either. We might need more experience with it.

@ahl
Copy link
Collaborator Author

ahl commented Jul 8, 2020

Thanks, @steveklabnik and @davepacheco.

Agreed with all your comments regarding the utility of these.

  • make this a const in HttpTypedResponse? I think this is what you were suggesting above. This way, each specific implementation would specify help text for it ("creation successful"). Presumably this would get plumbed through ApiEndpointResponse.
  • an attribute on the HttpTypedResponse type? This is pretty similar but might be more ergonomic?

Yeah, I was thinking something like that, but maybe with some ability for the developer to influence the output, potentially with some attribute on the type parameter.

  • the "endpoint" macro as you suggested: seems fine, agree it's not awesome

This feels like a decent fallback or override to keep in mind.

@ahl
Copy link
Collaborator Author

ahl commented Jul 8, 2020

Repurposing this issue to cover the implementation of the solution described above

@ahl ahl changed the title openapi: figure out how we want to handle response descriptions openapi: implement response descriptions Jul 8, 2020
@ahl ahl self-assigned this Aug 29, 2020
@ahl ahl closed this as completed in #50 Aug 31, 2020
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 a pull request may close this issue.

3 participants