Migrate legacy setup to default profile #259
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| // Only create a default profile if there are existing installed servers | ||
| if len(registry.ServerNames()) > 0 { |
There was a problem hiding this comment.
Question
I think I have been benefiting from a happy accident where I've been able to add/remove Atlassian for example, and then my config just pops back up and keeps working, but I suppose with profiles, since the config is scoped at the server we can't add the config to a server that doesn't exist?
There was a problem hiding this comment.
Right. That will be one tradeoff with the profiles model. If that becomes a desired "feature" we could brainstorm some ideas.
pkg/migrate/migrate.go
Outdated
| return ®istry, cfg, &tools, &mcpCatalog, nil | ||
| } | ||
|
|
||
| func deleteLegacyFiles() error { |
There was a problem hiding this comment.
Thought
Would it be better to archive these in a folder? If we somehow got this wrong and people complained we might be able to recover if we have the original files.
There was a problem hiding this comment.
Good point. I will update this.
pkg/migrate/migrate.go
Outdated
| _ = os.Remove(catalogIndexPath) | ||
| _ = os.Remove(oldCatalogPath) | ||
|
|
||
| // We use os.Remove to remove the directory, so only it's only removed if empty |
There was a problem hiding this comment.
nit typo in comment "only it's only"
|
Question How do we handle client connections that have |
|
@bobbyhouse Good callout. That is handled here.
|
What I did
Implemented a migration path from the existing default
registry.yaml,config.yaml,tools.yamlto adefaultprofile.Migrates when the profiles feature flag is on, only once ever (unless the db is deleted), either when:
--profileflagcatalog-nextcommand is runprofilecommand is runOnce the migration is successful, it moves the legacy files to
~/.docker/mcp/.backup.Follow up needed: We'll need to migrate
remoteservers as well once we've implemented remote support in profiles.Related issue
(not mandatory) A picture of a cute animal, if possible in relation to what you did