-
Notifications
You must be signed in to change notification settings - Fork 5
Introduce configuration version 2 #208
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
Conversation
…bump" This reverts commit 7f5d41b.
94c84f1
to
26cee2e
Compare
Co-authored-by: Daniel Harvey <[email protected]>
Co-authored-by: Daniel Harvey <[email protected]>
…tgres versions?" This reverts commit 0796fb9.
@@ -15,7 +15,7 @@ | |||
}, | |||
"ndc-postgres-12": { | |||
"feature": "postgres", | |||
"filter": "not test(configure_initial_configuration_is_unchanged) and not test(configure_is_idempotent)", | |||
"filter": "not test(postgres_current_only)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@danieljharvey : How does this look to you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great - naming it like this explains why we're skipping too which is nice.
@@ -1,32 +1,46 @@ | |||
//! Tests that configuration generation has not changed. | |||
//! | |||
//! If you have changed it intentionally, run `just generate-chinook-configuration`. | |||
//! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Yeah this should stop confusion in future, nice!
What
This PR adds a new version (
"2"
) of the deployment configuration data format.This version of the configuration is capable of expressing array types in collections and arguments.
Since this is the first time a new version is introduced there are a lot of changes the only purpose of which is to distinguish between versions.
Only the infrastructure-related shell of the connector is aware of different versions of deployment configurations existing. The core of the connector only works with a single internal version.
This PR is also the one to introduce tests of array types. In hindsight this ought to have been possible in the previous PR that introduced the internal types and transformations (#191).
Note that there is not yet any automated way to upgrade a configuration to a newer version, but this will be introduced shortly.
This PR also adds a changelog entry.
How
The file
version2.rs
is a duplicate ofversion1.rs
, which has been adapted to use the new data types (incidentally these are just the ones of the internal model).configuration.sql
now exists asversion1.sql
andversion2.sql
respectively, since these have different capabilities. This is becauseversion2.sql
introduces the ability to introspect array types.configuration.rs
now exposesRawConfiguration
andConfiguration
types which are enums of all the supported versions (currently 1 and "2").One big wart on the implementation is that serde and schemars are unable to derive trait implementations for these types correctly, since they only support strings as enum tags, and we used a number literal for version 1.
Once we drop support of version 1 completely we can remove the manually implemented instances.
The various
Connector
trait implementations now explicitly work on the internal representation of a configuration,RuntimeConfiguration
.