thema icon indicating copy to clipboard operation
thema copied to clipboard

Change `#Lineage.joinSchema` from `_` (top) to open struct with version

Open sdboyer opened this issue 2 years ago • 3 comments

#Lineage.joinSchema defines the join/least upper bound that must be maintained by all schemas in a lineage. Currently, it starts as top, _:

https://github.com/grafana/thema/blob/b7b433093f2fd56c11e68a45b0c3ee7b7850ca8c/lineage.cue#L11-L24

This isn't great:

  • It makes injecting a themaVersion field complicated, since we can't universally assume all schemas are a struct at base (could be scalar or list)
    • We can't even assume that all schemas are structs within a lineage
  • It's plausible that other bits of thema's helper CUE logic may be subtly assuming a struct type here as well
  • I don't see a use case for scalars or lists as a base schema type, for essentially the same reasons that

I'm sure i want it to be an open struct, {...}. i was worried about open/closedness here, but after some poking on the playground, i think we can make the shift without forcing any decisions on openness, which is great - that's a can of worms.

I don't see an immediate reason why this change would be anything other than changing just the one line - i'm mostly writing this issue up for posterity and as a place to dump that link to the playground. (also have it in a gist)

sdboyer avatar Jan 17 '22 20:01 sdboyer

Thinking on this further, it may be that we want to actually inject this instead:

joinSchema: {
    themaVersion?: [int & >= 0, int & >= 0]
    ...
}

But i'll probably leave that for a follow-up

sdboyer avatar Jan 17 '22 21:01 sdboyer

When talking all this through with @Lyoness and @myitcv earlier today, i realized that this, the possibility of adding the themaVersion? field, and #76 can be taken care of all in one motion by something like this:

  1. Making joinSchema into {...} as this originally proposed, and renaming it to `allSchem
  2. Adding an allSchemasMust, which is a struct that must be valid when unified against all schemas in the lineage, but the result of that unification is not part of the actual used schema.

sdboyer avatar Oct 26 '22 19:10 sdboyer