feat(tern): inject MySQL namespace per Spirit operation for namespace-free targets#340
Draft
aparajon wants to merge 1 commit into
Draft
feat(tern): inject MySQL namespace per Spirit operation for namespace-free targets#340aparajon wants to merge 1 commit into
aparajon wants to merge 1 commit into
Conversation
…-free targets Move MySQL schema selection out of target resolution and into LocalClient execution. A target DSN that already names a database keeps the local-DSN behavior, where the namespace is an organizational label and Spirit connects to the DSN's database. A namespace-free target DSN is the data-plane shape: the concrete namespace is the connection schema and is injected per Spirit operation at plan, pull, apply, stop, cutover, control setup, and resume. This lets a data-plane caller cache one namespace-free client per target while each schema change connects to its own namespace, and keeps schema selection out of the connection/target contract. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR updates tern.LocalClient to support MySQL “namespace-free” target DSNs by injecting the concrete MySQL schema (namespace) into engine credentials per operation, while preserving current behavior when the DSN already includes a database.
Changes:
- Added namespace-aware MySQL credential resolution utilities (inject schema into DSN only when the target DSN has no database).
- Wired those credentials through Spirit operations (plan, pull schema, apply, cutover/stop/control, and resume paths).
- Updated/expanded unit tests to cover DSN-shape dual-mode behavior and ensure tasks carry namespaces where required.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/tern/local_client.go | Adds MySQL namespace-aware credential helpers; updates Plan/PullSchema to use per-namespace DSN injection and supports multi-namespace planning for namespace-free DSNs. |
| pkg/tern/local_apply_sequential.go | Ensures sequential per-task engine calls use namespace-injected credentials for MySQL. |
| pkg/tern/local_apply_grouped.go | Ensures grouped/atomic applies use namespace-aware credentials derived from the plan. |
| pkg/tern/local_control.go | Routes cutover/stop/control setup and progress snapshotting through namespace-aware credentials. |
| pkg/tern/local_control_resume.go | Updates resume/start flows to resolve credentials using task/plan namespace where applicable. |
| pkg/tern/local_client_test.go | Adds targeted tests for DSN-shape dual-mode and namespace injection behavior. |
| pkg/tern/local_control_test.go | Updates control tests to provide namespace-free DSN inputs and task namespaces for MySQL paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+299
to
+312
| func (c *LocalClient) credentialsForGroupedApply(plan *storage.Plan) (*engine.Credentials, error) { | ||
| if c.config.Type != storage.DatabaseTypeMySQL { | ||
| return c.credentials(), nil | ||
| } | ||
| creds := c.credentials() | ||
| for namespace := range plan.Namespaces { | ||
| nsCreds, err := c.credentialsForMySQLNamespace(namespace) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| creds = nsCreds | ||
| } | ||
| return creds, nil | ||
| } |
Comment on lines
+577
to
+581
| creds, err := c.credentialsForApply(tasks, targetApplyID) | ||
| if err != nil { | ||
| c.logger.Warn("failed to resolve credentials for engine progress snapshot after stop", | ||
| "database", c.config.Database, "type", c.config.Type, "error", err) | ||
| return nil |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Why
A data-plane caller serves many targets through one cached client per target. For that, the target connection cannot carry the MySQL schema name — schema selection has to happen at execution time from the plan/task namespace.
What
Risk
Low. Existing configs whose DSN includes a database are unchanged; only namespace-free DSNs take the new injection path. This is a leaf of the larger data-plane routing work in #339.
🤖 Generated with Claude Code