Let's talk configuration!

slifty commented 4 years ago

Discussion

What do you want to talk about?

The long term vision for configuration would allow for things like API and UX based configuration.

The short term vision for configuration is necessarily far simpler. This issue is about the short term.

This issue is about fleshing out the contents of a config.json which will allow someone to set up:

Ingestion streams
A topology of appliances

This list may well expand.

Relevant Resources / Research

None

slifty commented 4 years ago

How's this for a proposed format for the data we want right now:

config.json

{
  "sources": [
    {
      "ingestionEngine": "",
      "settings": {},
      "metadata": {},
    }
  ],
  "appliances": [
    {
      "package": "",
      "name": "",
      "settings": {},
    }
  ]
}

This plan would have some secondary impacts:

We might want to create the concept of ingestion engine names (or maybe we would want to just use the class name of the ingestion engine?)
We should consider fleshing out the metadata associated with a stream (for now we can just skip that field / not formalize it yet -- we don't have to block on this)
Tweak to AbstractIngestionEngine API (see settings note below)

Appliance settings would be up to the individual appliance to specify; the field would just be a key / value store that is passed to the appliance's constructor in the settings parameter.

IngestionEngine settings would probably need to be similar (and we'd have to update our IngestionEngine to take in a settings parameter that is parsed.)

Not included

This does not propose anything related to configuration outside of the immediate implementation path (e.g. some day we might have configuration related to server names, ports, identifying information regarding the TV Kitchen host organization, etc).

Also as alluded to earlier the metadata field related to streams should be much more explicit. For instance appliances are probably going to benefit from knowing the metadata associated with a given stream.

slifty commented 4 years ago

Note: I toyed with the idea of a version as a settings field in appliances.

This would naturally be something we remove depending on where things land with #65.

Really it feels more appropriate to rely on package.json to specify appliance version, and the configuration to simply say which appliance we want (using eslint's api as a model for plugins, which appliances are functionally serving as.)

reefdog commented 4 years ago

Appliance settings would be up to the individual appliance to specify; the field would just be a key / value store that is passed to the appliance's constructor in the settings parameter.

Strikes me that verifying the format/contents of these settings is another thing the appliance's doctor should do.

Note: I toyed with the idea of a version as a settings field in appliances.

Hard agree with the rest of your comment that this feels like a dangerous anti-pattern that means we're replicating a package manager. I'd say if we find ourselves needing this, we should re-evaluate the path we've chosen. But, I do get that instance configurations / a particular topology may be reliant on a specific version of an appliance. (Like, it might mean that recipes themselves need to be proper apps with package.json files?) Anyway the closer config.json gets to "package.json outside of package.json", the more I squint.

slifty commented 4 years ago

Strikes me that verifying the format/contents of these settings is another thing the appliance's doctor should do.

Doctor would be run outside of the context of configuration of an appliance, so I'm not sure about this.

e.g. doctor would be run (ideally as a triggered part of yarn add) to just say "are all the appliance pre-requisites installed?" -- regardless of whether the appliance is being used in a given recipe (which is where configuration would occur)

I'm not sure settings need a validation method -- if the config is invalid it will either not work as expected OR it will fail to construct because a required setting was not provided. Do other projects with dynamic setting objects have a validation mechanism? I guess travis has travis lint for instance. (either way I think that would be a separate thing, not a doctor thing)

slifty commented 4 years ago

We have moved down a path of using the countertop API to add appliances (see #82)

In particular, configuration is going to happen in TVK instances -- where the countertop is imported as a package (package TBD #80) and coded against, as opposed to configuration happening directly on a clone of this repository.

tvkitchen / countertop