Skip to content

feat: add ListMysqlDatabases endpoint #113

Draft
VoLKyyyOG wants to merge 1 commit into
mainfrom
akiraw/ListMysqlDatabases
Draft

feat: add ListMysqlDatabases endpoint #113
VoLKyyyOG wants to merge 1 commit into
mainfrom
akiraw/ListMysqlDatabases

Conversation

@VoLKyyyOG

@VoLKyyyOG VoLKyyyOG commented May 18, 2026

Copy link
Copy Markdown
Collaborator

Create new endpoint for use by metanexus.
This lists all mysql databases.

ref: https://linear.app/squareup/issue/DFNS-445/create-schemabot-endpoint-listmysqldatabases

Copilot AI review requested due to automatic review settings May 18, 2026 02:04
@VoLKyyyOG VoLKyyyOG self-assigned this May 18, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new API endpoint for listing configured MySQL databases from SchemaBot’s server configuration.

Changes:

  • Defines response types for MySQL database inventory.
  • Registers and implements GET /api/mysql/databases with optional environment filtering.
  • Adds handler tests and updates API README route documentation.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
pkg/apitypes/apitypes.go Adds response structs for MySQL database inventory.
pkg/api/service.go Registers the new MySQL database listing route.
pkg/api/README.md Documents the new route and handler file.
pkg/api/mysql_handlers.go Implements the MySQL database inventory handler and filtering logic.
pkg/api/mysql_handlers_test.go Adds tests for listing, filtering, and empty configuration behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@VoLKyyyOG VoLKyyyOG marked this pull request as draft May 18, 2026 02:14
@aparajon

Copy link
Copy Markdown
Collaborator

Triage review of this draft, which has been open a while — sharing an honest assessment of where it stands relative to the current codebase:

  1. The API shape conflicts with the engine-agnostic surface rulepkg/apitypes and the route are MySQL-specific (/api/mysql/databases, MysqlDatabaseResponse), but the API layer is required to stay engine-agnostic; a database listing should be a generic GET /api/databases registry view that works for all engine types, with engine-specific data in metadata.

  2. Correctness: the Deployment field is populated with the database name — deployment is per-environment configuration, so consumers would receive fabricated routing metadata.

  3. Staleness — the branch is ~174 commits behind main and predates the per-environment deployments map, which the response type can't represent. There's also an exported handler wrapper that's a pure delegation (the repo avoids those).

  4. PR description — the body references an internal service and an internal issue tracker link; per repo policy those need to be removed from public PR descriptions regardless of outcome.

Given the schema-pull/onboarding primitives now landing (#313), the recommendation is to close this and re-approach the need as an engine-agnostic database listing on top of that work. Happy to discuss if there's a use case the generic shape wouldn't cover.


🤖 This review was generated by Claude Code (claude-fable-5) with maintainer approval.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants