elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.68k stars 8.23k forks source link

[Dashboards][Discuss] Public API URL path #193758

Open nickpeihl opened 2 months ago

nickpeihl commented 2 months ago

Prior to making the Dashboards Public API available, we should decide on how best to organize the base URL path for the CRUD routes.

Two suggested base paths are

  1. /api/dashboards
  2. /api/dashboards/dashboard

The expected CRUD routes for Dashboards use the following HTTP verbs and URL templates.

Create POST <BASEPATH>/{id?} - creates a new Dashboard using the attributes in the request body. In the URL id is the optionally specified saved object id. If id is not provided, then a random uuid is used.

Read GET <BASEPATH>/{id} - retrieves a Dashboard with the matching saved object id.

Update PUT <BASEPATH>/{id} - updates an existing Dashboard using the attributes in the request body. The id in the URL is required and must match an existing saved object.

Delete DELETE <BASEPATH>/{id} - deletes an existing Dashboard that matches the provided saved object id.

List GET <BASEPATH> - retrieves a paginated list of Dashboards. The URL may contain a query string that can be used to filter dashboards.

Kibana has a mixed usage when it comes to organizing API paths for CRUD:

elasticmachine commented 2 months ago

Pinging @elastic/kibana-presentation (Team:Presentation)

timductive commented 1 month ago

My preference would be to start standardizing on: /api/{pluralnoun}

So in this case: /api/dashboards

GET /api/dashboards should be the route to return the list of dashboards.

This doesn't prevent a future use of: POST/PUT /api/dashboards/config or POST/PUT /api/dashboards/default

This is the common usage pattern I've seen in the industry and avoids the redundancy of identifying a singularnoun dashboard in every api request.

nickpeihl commented 1 month ago

This doesn't prevent a future use of: POST/PUT /api/dashboards/config or POST/PUT /api/dashboards/default

True. My primary concern with this is a very strict edge case. Consider that we allow users to optionally specify the saved object ID of their dashboard at POST /api/dashboards/<id>. At some point maybe a user creates their dashboard with a saved object ID of default, e.g. curl -XPOST "/api/dashboards/default" ... -d { "attributes": { ... } }.

Later we decide to add a default subpath that allows users to set default dashboards in the dashboards app. The request and response schemas at the /api/dashboards/default endpoint have suddenly changed. The user who naively used default as a saved object ID can no longer perform CRUD operations on their dashboard. They can still use the UI to perform CRUD operations since the UI uses the internal RPC endpoints.

That said, I do like the shorter and REST-like path you propose. But I wonder if that should restrict us from creating any other paths (e.g. config, default) under the dashboards domain? In that case, maybe we would introduce a different endpoint for configuring the Dashboards app (e.g. /api/dashboards-management/[config, default, etc]).

I guess it really comes down to the question, does dashboards in /api/dashboards refer to the Dashboards app or a collection of dashboards?

Heenawter commented 1 month ago

Could we not have /api/dashboards as the route to list all dashboards, while keeping /api/dashboards/dashboard/<id> as the route for creating/updating dashboards? So, anything under api/dashboards refers to a collection of dashboards - and then once you are under api/dashboards/dashboard, you are referring to a single dashboard.

To me, this pattern still makes sense and removes the edge case you mentioned @nickpeihl 🤔 But not sure if we want to be that explicit or if it's better to keep everything under the same level.

nickpeihl commented 1 month ago

Could we not have /api/dashboards as the route to list all dashboards, while keeping /api/dashboards/dashboard/<id> as the route for creating/updating dashboards? So, anything under api/dashboards refers to a collection of dashboards - and then once you are under api/dashboards/dashboard, you are referring to a single dashboard.

IMO, this further muddles the question of what is dashboards exactly? If we consider dashboards to be the Dashboards App, then listing the Collection of dashboard items at /api/dashboards conflates the App with the Collection. The CRUD operations at /api/dashboards/dashboard/<id?> implicitly suggests "dashboards" is the App and "dashboard" is the Collection.

lukeelmers commented 1 month ago

I guess it really comes down to the question, does dashboards in /api/dashboards refer to the Dashboards app or a collection of dashboards?

I would suggest keeping our API path names focused on resources rather than individual applications. Applications are somewhat malleable, but resources generally aren't. This is especially true for platform-owned components. (e.g. what if we start including dashboards inside other applications in the future? would APIs need to change to /api/some-other-app/dashboard?)

A focus on applications also risks us designing our APIs around the org structure, when for users our entire platform API surface area should ideally feel like one product built by one team.

My general sense here is that we probably shouldn't overthink this... simpler is usually better, and changing path names in the future is a low maintenance burden (i.e. we can easily register the same route handler for multiple paths if needed)

cc @kobelb for his input

kobelb commented 1 month ago

I agree with Luke. We should keep this simple for the time being, and not try to future proof ourselves.

My preference is that we use /api/dashboard (singular) as the path. This breaks from industry-wide convention, but matches the Elasticsearch convention:

Model names within the path should be in singular form, not pluralized, even when the endpoint is capable of returning more than one of the model.

If in the future, we want a HTTP API for dashboard config, this leaves us with two options.

1) Follow the ES conventions again, and introduce a "dashboard" namespace, which would be prefixed with a _, leading to a /api/_dashboard/config path. We could either continue to use /api/dashboard for the dashboard resource, or also include it in the _dashboard namespace and use /api/_dashboard/dashboard (a bit more awkward IMO). 2) No longer follow the ES conventions, and introduce a "dashboards" namespace, not prefixed by a _, leading to /api/dashboards/config. We could then continue to use /api/dashboard for the dashboard resource, or use the "dashboards" namespace and use /api/dashboards/dashboard.

A similar approach could be followed if we want to set a default dashboard.