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

thema: Flatten schemas to one-dimensional array #82

Merged
merged 60 commits into from
May 2, 2023
Merged

Conversation

sdboyer
Copy link
Contributor

@sdboyer sdboyer commented Oct 31, 2022

After a suggestion from @myitcv at Kubecon last week, the light finally clicked on with how we can get rid of the nested array in seqs, add reverse lenses to the structure, and overall just drastically improve UX. In fact, we can get rid of the concept of "sequence" entirely.

As an example, the final form for the narrowing exemplar is this:

schemas: [{
	version: [0, 0]
	schema: boolish: "true" | "false" | bool | string
}, {
	version: [1, 0]
	schema: properbool: bool
}]

lenses: [{
	to: [0, 0]
	from: [1, 0]
	input: _
	result: {
		// Preserving precise original form is a non-goal of thema in general.
		boolish: input.properbool
	}
	lacunas: []
}, {
	to: [1, 0]
	from: [0, 0]
	input: _
	result: {
		if ((input.boolish & string) != _|_) {
			properbool: input.boolish == "true"
		}
		if ((input.boolish & bool) != _|_) {
			properbool: input.boolish
		}
	}
	lacunas: [
		{
			sourceFields: [{
				path:  "boolish"
				value: input.boolish
			}]
			targetFields: [{
				path:  "properbool"
				value: result.properbool
			}]
			message: "boolish was a string but neither \"true\" nor \"false\"; fallback to treating as false"
			type:    thema.#LacunaTypes.LossyFieldMapping
			cond:    ((input.boolish & string) != _|_) && ((input.boolish & ("true" | "false")) == _|_)
		},
	]
}]

seqs is gone, nested arrays are gone. version is an explicit field within the #SchemaDef (though there's still no actual choice to be made by the user - it's all governed by backwards compat rules) Additional fields are possible in there, including providing examples of the schema.

Lenses are moved into their own, separate structure, independent of the schemas list. They're a bit more verbose than i'd like, with it still be necessary to explicitly write the input field. But from and to are now just a version number, rather than needing to be a reference to the correct schema. Better names for the other fields, too - i think result is indicative of what's supposed to go there, though i don't think this has reached the level of self-explanatory quite yet.

Fixes #2
Fixes #18

  • Add native CUE check that lens set is complete
  • Add more native CUE test cases
  • Convert existing exemplars, tests
  • Refactor Go packages to work with new CUE structures
  • Introduce AST-based migrator

@sdboyer sdboyer marked this pull request as ready for review May 2, 2023 04:28
@sdboyer
Copy link
Contributor Author

sdboyer commented May 2, 2023

This is finally, finally good to go. Lots of follow-ups, of course.

@sdboyer sdboyer merged commit 3b4fcc2 into main May 2, 2023
2 checks passed
sdboyer pushed a commit that referenced this pull request May 8, 2023
This is a reversion of a change made in #82. We'll be switching back to
to using `any` as soon as possible, but for now it's being pulled out in
order to reduce the number of simultaneous changes we need to manage in
grafana/grafana.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Breaks existing code enhancement New feature or request
Projects
None yet
2 participants