SY-4360: Task Config Provider Registry and Wire Composition#2472
SY-4360: Task Config Provider Registry and Wire Composition#2472emilbon99 wants to merge 1 commit into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## rc #2472 +/- ##
==========================================
+ Coverage 67.60% 67.64% +0.03%
==========================================
Files 2528 2529 +1
Lines 121829 122061 +232
Branches 8995 9011 +16
==========================================
+ Hits 82364 82569 +205
- Misses 33030 33049 +19
- Partials 6435 6443 +8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| p, ok := w.providers.get(res.Type) | ||
| if !ok { | ||
| return res, nil | ||
| } | ||
| configID, err := p.Copy(ctx, w.tx, key, newKey) | ||
| if err != nil { | ||
| return Task{}, err | ||
| } | ||
| if err = w.otg.DefineRelationship( | ||
| ctx, | ||
| OntologyID(newKey), | ||
| ontology.RelationshipTypeParentOf, | ||
| configID, | ||
| ); err != nil { | ||
| return Task{}, err | ||
| } | ||
| return res, nil |
There was a problem hiding this comment.
Copy doesn't handle pre-bootstrap ErrNotFound from provider
ResolveConfigs explicitly swallows query.ErrNotFound from p.Load, so pre-bootstrap tasks (those whose config hasn't been migrated to a provider yet) resolve correctly. Copy has no equivalent fallback: if a provider is registered and the source task has no config record yet, p.Copy returns query.ErrNotFound, and this surfaces as an error from Writer.Copy. During a live migration window — where some tasks have config records and others don't — copying an unbootstrapped task will fail even though it would succeed for tasks without a registered provider.
1 participant
Issue Pull Request
Linear Issue
SY-4360
Description
Phase 1 of the task/config split (RFC 0042, #2471): the type-agnostic machinery that
lets per-integration config services plug into the task service. Inert in production:
no providers are registered, so behavior is byte-identical to today.
ConfigProviderinterface + registry (core/pkg/service/task/config.go).Providers register exact task-type strings (mirroring imex's per-subtype importers);
duplicate registration errors. All provider methods receive the task writer's
gorp.Tx, so a task and its config are created, copied, or deleted in onetransaction.
Createroutes the effective config (including thesnapshot-preserved config on updates) to the matching provider and defines the
task -> parent -> configontology relationship.DeleteandCopyroute throughthe provider. Internal tasks and task types without a provider take the existing
embedded-blob path unchanged. The blob keeps being written alongside the typed
record (dual-write), per the RFC's downgrade-safe rollout.
task.Service.ResolveConfigsfillsTask.Configfrom theprovider on the API retrieve path, falling back to the stored blob when the provider
has no record, so mixed states (pre-bootstrap tasks) resolve correctly.
DecodeConfighelper. The newest-first decode chain providers will use tonormalize old-shaped wire input through the same versioned transforms as storage
migration.
Covered by a fake-provider Ginkgo suite (registration conflicts, dispatch, internal
exclusion, fallback, snapshot preservation, delete/copy routing, resolution fallback,
decode chain). Task, driver, and rack suites pass.
Basic Readiness
Greptile Summary
This PR introduces Phase 1 of the task/config split (RFC 0042): a
ConfigProviderinterface and registry that lets per-integration config services plug into the task service, with writer dispatch for Create/Delete/Copy and aResolveConfigsread path that fillsTask.Configfrom the provider, falling back to the stored blob for unregistered types and pre-bootstrap tasks. Because no providers are registered yet, runtime behavior is byte-identical to the current state.ConfigProvider+ registry (config.go): thread-safe registration with duplicate-type detection;ResolveConfigsskips internal tasks, unknown types, and tasks whose provider returnsErrNotFound, correctly handling mixed migration states.writer.go):Createcaptures the snapshot-preserved config before gorp write and routes it to the provider;Deletepre-fetches the task to determine its type and internal flag before dispatching;Copylinks the duplicated config resource to the new task's ontology node.DecodeConfighelper (config.go): generic newest-first decode chain returning the first success or the first (current-schema) error, withErrNoConfigDecodersfor the zero-decoder case; backed by a thorough Ginkgo suite.Confidence Score: 4/5
Safe to merge as written — no providers are registered in production yet, so all new dispatch paths are unreachable and runtime behavior is unchanged.
The writer's Copy path has no fallback for query.ErrNotFound from p.Copy, unlike ResolveConfigs which explicitly handles pre-bootstrap tasks. Once providers go live in a later phase, copying an unbootstrapped source task will fail rather than silently falling back to the stored blob.
core/pkg/service/task/writer.go — specifically the Copy function's provider dispatch block.
Important Files Changed
Sequence Diagram
sequenceDiagram participant API as api/task participant W as Writer participant T as gorp.Table participant P as ConfigProvider participant O as Ontology note over API,O: Create / Update path API->>W: Create(ctx, task) W->>T: NewCreate + MergeExisting (snapshot cfg preserved) T-->>W: merged task (cfg captured) W->>O: HasResource(taskID) alt first creation W->>O: DefineResource(taskID) W->>O: DefineRelationship(group → task) end alt "provider registered && not internal" W->>P: Create(ctx, tx, key, type, cfg) P-->>W: configID W->>O: DefineRelationship(task → config) end note over API,O: Retrieve path (ResolveConfigs) API->>W: Retrieve query W-->>API: []Task (with stored blobs) API->>P: ResolveConfigs(ctx, nil, tasks) loop each non-internal task with provider P->>P: Load(ctx, tx, key) alt found P-->>API: cfg (overrides stored blob) else ErrNotFound note over API: fallback: keep stored blob end end note over API,O: Delete path API->>W: Delete(ctx, key, allowInternal) W->>T: Retrieve(key) → task alt "task found && not internal && provider exists" W->>P: Delete(ctx, tx, key) end W->>T: Delete(key) W->>O: DeleteResource(taskID) note over API,O: Copy path API->>W: Copy(ctx, key, name, snapshot) W->>T: NewUpdate (clones entry with newKey) W->>O: DefineResource(newTaskID) alt "not internal && provider exists" W->>P: Copy(ctx, tx, from, to) P-->>W: configID W->>O: DefineRelationship(newTask → config) endReviews (1): Last reviewed commit: "SY-4360: Task config provider registry a..." | Re-trigger Greptile