search

tvkitchen / countertop

The entry point for developers who want to set up a TV Kitchen.
https://tv.kitchen
GNU Lesser General Public License v3.0
6 stars 2 forks source link

Payload class versions #127

Open slifty opened 3 years ago

slifty commented 3 years ago

Discussion

What do you want to talk about?

We have a @tvkitchen/base-classes package which defines some common classes for use across the project.

However, Payload objects are generated by appliances. This means the countertop doesn't get to determine the exact type being passed unless it were to re-package payloads in new objects.

This prompts a few questions:

  1. Does this matter?

  2. If so, what should be done?

slifty commented 3 years ago

Does this matter?

It could be the responsibility of the appliance author to update to use the latest Payload / Payload array as new versions of the base-class package comes out. The issue is that it's hard to debug a topology where some appliances are generating "bad" payloads, and a given appliance might work in one topology and not in another depending on whether a given stream connects appliances with invalid payload / payloadarray versions.

I think it matters enough to at least document / come to a decision.

What should be done?

Repackaging in the countertop

We could have the countertop re-package into a common Payload type. In a away this already happens because we use AvroPayload to serialize -- so this is more meta / what happens if Payload evolves in a way that AvroPayload is not able to serialize certain types of payload.

(Also of note: IAppliance and AbstractAppliance face a similar version of this situation with PayloadArray since Appliances are expected to return a PayloadArray and the versions may differ.)

Injecting the Payload / PayloadArray classes via Appliance construction

This has an added benefit that Appliances wouldn't need to add Payload / PayloadArray as a package dependency.

The negative is that if features are removed from Payload or PayloadArray then a given appliance relying on that injection might break between major releases. Even so, that's better than the current state (appliances potentially breaking a topology on minor releases because they no longer generate Payloads with the latest features)

slifty commented 3 years ago

Right now I'm leaning towards:

  1. Updating the IAppliance interface to return arrays of Payloads instead of a PayloadArray

  2. Updating the IAppliance interface (and AbstractAppliance) to accept injected base classes.