-
Notifications
You must be signed in to change notification settings - Fork 134
Add server search feature to workingsets #211
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
Add ability to search across all working sets to return a list of servers grouped by working set id.
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.
lgtm. One minor nit, but not a blocker.
| WHERE ($1 = '' OR id = $1) | ||
| AND ($2 = '' OR EXISTS ( | ||
| SELECT 1 | ||
| FROM json_each(servers) | ||
| WHERE LOWER(json_extract(value, '$.image')) LIKE '%' || LOWER($2) || '%' | ||
| OR LOWER(json_extract(value, '$.source')) LIKE '%' || LOWER($2) || '%' | ||
| )) |
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.
SQL ninja! 🥷
pkg/workingset/search.go
Outdated
| Servers []Server `json:"servers" yaml:"servers"` | ||
| } | ||
|
|
||
| func Search(ctx context.Context, dao db.DAO, query string, workingSetID string, format OutputFormat) error { |
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.
nitpick: In case we ever add a docker mcp workingset search, maybe we should rename this to SearchServers?
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.
I think me naming it Search here breaks convention of keeping it the same name as the command. I'll rename to Servers and will free up Search
| } | ||
| } | ||
|
|
||
| formatting.PrettyPrintTable(rows, []int{40, 10, 120}) |
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.
Oh, I should use this on the other human readable prints. Didn't realize we had it!
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.
We should probably factor it out. It is coming from
github.com/docker/mcp-gateway/cmd/docker-mcp/secret-management/formatting
What I did
Add ability to search across all working sets to return a list of servers grouped by working set id.
For example
docker mcp workingset servers --filter test --workingset test-setWould return