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

feature: Add Feature discovery API endpoint and plugin interface #1630

Merged
merged 27 commits into from
Apr 22, 2024

Conversation

panaaj
Copy link
Member

@panaaj panaaj commented Sep 20, 2023

Overview:

Implement an API and ServerPlugin interface method to enable server features to be discovered by WebApps and plugins.

Current State:

The server provides an HTTP endpoint /skServer/plugins to list information about the installed plugins on the server, but this endpoint requires the requestor to be authenticated (logged in) and limits a WebApp's ability to discover features supported by the serrver at start up.

Additionally there is currently no ability to retrieve a list of implemented APIs from the server.

Implementation:

This PR implements the following:

  1. An HTTP API endpoint which can be called without the need for the requestor to be authenticated.
  • /signalk/v2/features
  1. A server Plugin interface method:
  • app.getFeatures(enabled?: boolean)

The caller can request that:

  • All features are returned
  • Only the features / plugins that are enabled are returned
  • Only the features / plugins that are disabled are returned

Example requests:

Example: Return ALL features.

// Using HTTP API:
HTTP GET "/signalk/v2/features"  

// Using plugin interface:
`app.getFeatures()

Example: Return only ENABLED features.

// Using HTTP API:
HTTP GET "/signalk/v2/features?enabled=true"  

// Using plugin interface:
`app.getFeatures(true)

Example: Return only DISABLED features.

// Using HTTP API:
HTTP GET "/signalk/v2/features?enabled=false"  

// Using plugin interface:
`app.getFeatures(false)

Example response:

{
  "apis": [
    "resources",
    "course"
  ],
  "plugins": [
    {
      "id": "course-provider",
      "name": "Course Data provider",
      "version": "1.0.0",
      "enabled": true
    },
    {
      "id": "freeboard-sk",
      "name": "Freeboard-SK",
      "version": "2.2.5",
      "enabled": true
    },
    {
      "id": "resources-provider",
      "name": "Resources Provider (built-in)",
      "version": "1.1.1",
      "enabled": true
    }
  ]
}

Features Scope

All APIs implemented by the server.
All Plugins installed on the server

Implement features API endpoint.
@panaaj panaaj changed the title [WIP] Add Features API endpoint and plugin interface feature: Add Feature discovery API endpoint and plugin interface Sep 27, 2023
@panaaj panaaj requested a review from tkurki March 22, 2024 05:28
@panaaj
Copy link
Member Author

panaaj commented Mar 24, 2024

@godind does this PR address feature discovery requirements for KIP?

@godind
Copy link
Contributor

godind commented Mar 24, 2024

You read minds! When we briefly talked about it, that was my first thought - we need something public.

It's just enough to know to block features and inform users.

Brilliant!

@panaaj
Copy link
Member Author

panaaj commented Mar 25, 2024

@tkurki Given this is more server related I am wondering whether feature discovery is better placed under the /skserver root rather than /signalk/v2/api?

packages/server-api/src/index.ts Outdated Show resolved Hide resolved
src/api/features/index.ts Outdated Show resolved Hide resolved
src/app.ts Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
@panaaj
Copy link
Member Author

panaaj commented Mar 31, 2024

@tkurki I have updated the PR so now:

  1. APIs are added to the "discovered" list when they are initiated / started.
  2. Plugins can set a feature attribute when they are implementing an API (e.g. anchor) to allow them to be "discovered" and the API to be added to the list.

With regards to plugins implementing APIs, currently the API id needs to match an entry in a list. The thinking here is to provide some mechanism of governance (and potentially alignment checking) for plugin implemented APIs.

Proposed API list is (as these have at least been implemented to some extent already or under construction):
[
'resources',
'course',
'history',
'autopilot',
'anchor',
'logbook'
]

@tkurki
Copy link
Member

tkurki commented Apr 7, 2024

Apologies for being so inactive here.

I've been mulling this over in the background. Some thoughts:

  1. Do we need a separate discovery endpoint for the http APIs, instead of the client being able to probe the actual http API? The proof is in the pudding - either the API works or it doesn't, probing a separate feature discovery API may or may not be in line with the actual API. If I take as an example History API, KIP and signalk-to-influxdb - would it not be simpler to query the History API and the UI adjust to the response? You'll need to do error handling anyway. Also if I get the author of signalk-to-mongodb to implement History API what added values does registering as a provider for History API provide?
  2. Continuing with the History API we have actually two history APis, one in the specification and one as a PR. For the earlier one, that provides history streaming and snapshots, we have support in the plugin API via registerHistoryProvider. The latter is just the plugin registering route handlers. Having a real plugin API to implement would allow plugin to plugin usage and uniform http parameter parsing so that the plugin could implement just a TS api. This is naturally more work, to get the API support into the server. "Feature registration" would be a byproduct of implementing and registering the API.
  3. The list of installed plugins and their status seems like a separate, useful matter. I think it is a good idea to provide that, both via http and for other plugins via ts API. Should we have getPlugins that works like the current getFeatures().plugins- the plugins are not really features, the caller needs to have pretty specific knowledge about what other plugins do to make use of the list of enabled / installed plugins

It may very well be that I just haven't bumped into the use cases for this API - could you elaborate a little more here? It is also a good idea to think about the API client: are we talking about webapps, other plugins or use from some other system? My point above about just probing the API holds for webapps. but not plugins. But then again we don't have a way for plugins to use OpenApi apis, so what can a plugin do with the information that the system has History API if it can not use the API? A practical example could be polar calculations, that could use historical data if it were available and a standard API would allow using any implementation backed by some database.

I'll promise to find the time to continue this conversation more actively. Again, apologies for the silence.

PS. I'll submit a bunch of individual review comments, apparently i started a review and never sent it earlier.

src/api/features/index.ts Outdated Show resolved Hide resolved
src/interfaces/plugins.ts Outdated Show resolved Hide resolved
src/api/features/index.ts Outdated Show resolved Hide resolved
src/api/features/index.ts Outdated Show resolved Hide resolved
src/interfaces/plugins.ts Outdated Show resolved Hide resolved
packages/server-api/src/index.ts Outdated Show resolved Hide resolved
src/api/index.ts Outdated Show resolved Hide resolved
src/api/index.ts Outdated Show resolved Hide resolved
@panaaj
Copy link
Member Author

panaaj commented Apr 9, 2024

  1. Do we need a separate discovery endpoint for the http APIs, instead of the client being able to probe the actual http API? T

The Signal K server already knows what is available, so why not just ask it?
One call, a simple and consistent way for all Clients to determine what features are present.

2. ...Having a real plugin API to implement would allow plugin to plugin usage and uniform http parameter parsing so that the plugin could implement just a TS api. This is naturally more work, to get the API support into the server. "Feature registration" would be a byproduct of implementing and registering the API.

I think we need to land on how Signal K defined APIs should be implemeted by plugins.

Clearly there are several approaches but I personally lean towards the API definition and route handling being done by the server and passed through to a registered plugin (much like the approach taken for resources and autopilot).

A good test case for options is an Anchor API.... there is already a plugin that has the required functionality but it lacks an API within the /signalk/v2/api namespace.

Do we:

  1. Just add the additional API paths and definitions to the plugin?
  2. Bring the operation into the server (like Course API)?
  3. Implement the API routes on the server and have the plugin register as a provider?

3. The list of installed plugins and their status seems like a separate, useful matter. I think it is a good idea to provide that, both via http and for other plugins via ts API.

I have no problem with them being separate, in reality a client shouldn't need to know what plugins are installed.... the features of interest should be in getFeatures

@panaaj
Copy link
Member Author

panaaj commented Apr 9, 2024

It may very well be that I just haven't bumped into the use cases for this API - could you elaborate a little more here?

  1. Trimming UI and features presented by the App
  2. Prompting the user with "server actions" required to make App feature available (e.g. install a particular plugin)
  3. Transition away from using "Proof of concept" feature (client implemented) to server implemented. This is particularly useful as Signal K is gowing through the move to version 2.

@tkurki tkurki merged commit abbc56c into master Apr 22, 2024
4 checks passed
@tkurki tkurki deleted the features_api branch April 22, 2024 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants