budibase icon indicating copy to clipboard operation
budibase copied to clipboard

Public API SDK

Open aptkingston opened this issue 2 years ago • 2 comments

Description

This PR adds a JS SDK for our public API. This SDK is entirely generated by swagger-codegen using the OpenAPI specs we generate.

The generated SDK uses promises and the generated methods have pretty names by way of the new operationId tag added to all endpoints.

Publishing

The swagger-codegen generator is a Java app. Since adding java as a dependency to dev stacks (and release workflows) would be a nightmare, the generator instead runs inside a docker container - meaning docker is the only dependency. The generated code is ignored and not checked in to the repo, but we have a custom entrypoint in src/index.js which exposes only the relevant APIs and a handy configuration method.

The SDK is not used by any other Budibase packages, so we can exclude it from the usual build and dev scripts. There's a new top level yarn script called build:sdk which will generate and bundle the SDK. This script has been added to only those release jobs which publish to NPM. I think they already have docker available to them inside the ubuntu-latest runner.

Stucture

The top level exports of the SDK are the available APIs, and a configuration utility. The full list is:

  • configure
  • ApplicationsApi
  • QueriestApi
  • RowsApi
  • TablesApi
  • UsersApi

These are all named exports, and the SDK is currently only build as an ES module, so importing things would look like: image

Code completion / IDE intellisense

As we aren't wrapping the generated endpoints, IDEs can take full advantage of the excellent generated docs and provide great code completion prompts: image

Configuration

The SDK is configured with only 2 properties:

  • API key (acquired from the Budibase portal or programmatically inside plugins)
  • Host (the server where your API is running - defaults to the current host)

The SDK instance is confgured using a simple configure utility. This is required before using any of the APIs, as otherwise authentication will fail. A simple example of authenticating would look like: image

The current public API does not properly configure CORS, so using a different host will prevent the SDK from working. I'm assuming this is just an oversight and we should add this? I'll follow up on this.

Usage inside component plugins

There is a new utility provided by the client SDK to make authentication easier inside plugins. There's a simple getAPIKey utility which will return the users API key, which can then be passed into the configure method.

A quick example would look like this. Note that we do not need to specify the host configuration parameter, as we expect the public API to be running on the same host that this component is being served from (which will always be the case for component plugins, unless you want to contact a different Budibase install from the one you're running on). image

The public API SDK is not shipped by the client library be default. This means it is opt-in to use it, and plugin authors can version it themselves.

Questions

  • The generated SDK includes WithHttpInfo suffixed versions of all API endpoints. This is unavoidable and a consequence of generating a promisified SDK (as opposed to a callback based one, which is the default). We cannot remove these functions as the normally named SDK functions call through to these. Is that an acceptable tradeoff that we just acknowledge they exist and say to just ignore them? You can see an example in the first screenshot above, where there is createRow and createRowWithHttpInfo.
  • With this being released, I think the current client library context (called sdk) needs to be renamed. The double usage of sdk is confusing. I would propose we rename the client library context to just client - i.e. const { styleable } = getContext("client)". Any thoughts on that?

Addresses

  • #6960

aptkingston avatar Sep 21 '22 08:09 aptkingston

Codecov Report

Merging #7866 (448c62e) into develop (3a6d009) will not change coverage. The diff coverage is n/a.

:exclamation: Current head 448c62e differs from pull request most recent head 96685f3. Consider uploading reports for the commit 96685f3 to get more accurate results

@@           Coverage Diff            @@
##           develop    #7866   +/-   ##
========================================
  Coverage    67.37%   67.37%           
========================================
  Files          123      123           
  Lines         4138     4138           
  Branches       660      660           
========================================
  Hits          2788     2788           
  Misses         946      946           
  Partials       404      404           

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

codecov-commenter avatar Sep 21 '22 09:09 codecov-commenter

On the comment:

The generated SDK includes WithHttpInfo suffixed versions of all API endpoints. This is unavoidable and a consequence of generating a promisified SDK (as opposed to a callback based one, which is the default).

Makes sense these can't be removed due to them being called through to by the SDK itself - I was just wondering if there is anyway we could remove the export of them however? If they weren't exported I think most IDEs will realise they aren't available functions and wouldn't be part of the code completion. If its not possible I think the benefit of being able to generate the SDK far outweighs the negative of a few poorly named/useless functions being exposed, we can just explain that in the documentation, certain functions ending in withHttpInfo are not to be used as they are for internal use only.

The other option might be to name they even more poorly - e.g. renaming all of the withHttpInfo to something very obviously not for external use like _int_api_search_rows - not using camel casing etc - looks very obvious that its not a useful function.

Overall I think this is amazing, so great to get this built out and good idea using the Docker container to generate rather than requiring all of the Java dependencies themselves!

mike12345567 avatar Sep 21 '22 11:09 mike12345567

It's mind blowing that this works almost out of the box, and will allow us to generate SDKs for other programming languages as well in future.

The generated SDK includes WithHttpInfo suffixed versions of all API endpoints.

It would be good to hide these if possible, if they provide no value. However are they definitely useless? Is there absolutely no chance you would ever use them? Even some weird case?

With this being released, I think the current client library context (called sdk) needs to be renamed.

Yeah to be honest it's not really an SDK as was originally planned - it's more just the internal API of the client library now so think this makes sense. client works, or maybe we could be more explicit and call it core - as that's really what it is.

They're definitely useless yea. They accept the exact same params as their equivalent, normal named counterparts. The only difference is that the WithHttpInfo suffix version actually does the logic, and the short-named variant literally just calls through to the long variant and follows it with a then(x => x.data). So they basically exist to just strip the .data property out of the response, which is completely pointless. They only do this when you generate a promisified JS SDK though, not the callback version.

I'll keep trying to see if there's a way to keep IDE code completion while hiding these (and keeping the logic generic so we don't need to ever update this as we update the public API), but so far I don't see a way to have it all.

aptkingston avatar Sep 21 '22 18:09 aptkingston

I've updated the description to show the improved syntax, following some of @Rory-Powell's suggestions. The new example usage would look like:

  import Budibase from "@budibase/sdk"

  const budibase = new Budibase({
    apiKey: "my-api-key",
    host: "https://bb.budibase.app",
  })

  await budibase.rows.create()
  await budibase.applications.search()
  await budibase.queries.execute()

This is easier to use, allows multiple clients, and follows the example of a lot of similar public SDKs, while still being zero maintenance over time. The only time we'll need to touch this SDK code is if we add a new module to the public API entirely (e.g. a groups API). Adding/updating/deleting endpoints within existing APIs requires no changes.

aptkingston avatar Sep 21 '22 18:09 aptkingston

This should be ready to merge whenever now - all the comments have been addressed :+1: More issues may come out of using it but we'll get a better idea once this is published to npm for the first time, since it'll be a lot easier to test in different environments (like inside a custom component).

aptkingston avatar Sep 26 '22 14:09 aptkingston