Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://hackmd.io/n2G9RlgCRrOZo_-M91NMJw?both
Problems / Missing pieces
Changes to provisioner code hasn't started
Provisioner used to be the one to create the deployment, but instead it needs to watch for changesets to provision. Buildengine now points to schema service to create a changeset, whereas it used to point to provisioner to call Create/ReplaceDeployment.
So currently when you run
ftl dev
it will get stuck:Chicken and egg problem with module schema and deployment key
I imagined this would happen:
CreateChangeset
call, creates a changeset with modules.But I think things are currently set up such that a module schema only gets a deployment key when it also has an endpoint (both are non optional fields of
ModuleRuntimeDeployment
).Assumption for now is that most services only care about canonical schema
So no calls will route to a deployment that is within a changeset that is still being provisioned. Same for cron, ingress, etc etc.
But pubsub breaks that assumption. As the runner becomes ready it will immediately start consuming events. Which can then start making calls which can do whatever...
I think for the first go at changesets that will just have to be a known issue that we fix afterwards.
GetSchema hasn't been updated with options for changesets
The idea was that we would make the GetSchema endpoint allow the caller to specify if they want to get a specific changeset in the response (use case: CI wanting to know if a changeset it kicked off was started)
Pull Schema is currently super cumbersome for most use cases
A lot of the callers of PullSchema are currently broken, and need to be reimplemented with the new events.
But I think the new events are overkill for their use cases. Given most services don't care about changesets, they really just want to care about updates to canonical deployments and when a changeset gets committed to canonical. But the problem is that the event for a changeset being commited does not currently include final module schemas for eveything within that module. So instead they would need to keep track of all the
ChangesetCreated
andDeploymentUpdated (with changeset key)
events just to have an accurate view of the changeset when it gets commited.So yeh... I think this should get simplified if we just send the whole contents of the deployments along with
ChangesetCommited
eventsOther gotchas / half finished thoughts:
schemaservice.GetDeployment( deploymentKey + changesetKey)
andschemaservice.FindDeployment(deploymentKey) (also returns an optional changeset key)
. Same goes for whether provisioner needs to include changeset key whenever it wants to update the schema as each resource is provisioned.FindDeployment()
. Maybe that's just growing pains of a refactor like this though. But i can see the advantanges of just using deployment key by itself everywhere.GetActiveDeployments
. i think I got a bit ahead of myself and started delineating betweenCanonical
views of the schema and cases where you'd want theLatest
view of the schema which replaces any canonical deployments with those that have been successfully deployed within a changeset.Latest
view of the schema because nothing should be calling those yet.DeploymentCreated
event. Changeset Commited events should now bundle those 2 separate events into one, so there should be some code we can simplify. eg: cron service.