feat(spanner): add spanner-get-database-ddl tool - #3818
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new Spanner tool, spanner-get-database-ddl, which retrieves the DDL statements defining the schema of a Cloud Spanner database. This includes adding the tool implementation, updating the Spanner source to support fetching the database DDL via a Spanner admin client, adding unit tests, and providing documentation. The reviewer noted an issue in the tool's Invoke method where authentication errors from ParseBearerToken are incorrectly wrapped and returned as internal server errors (500) instead of unauthorized errors (401), and suggested propagating the original error.
| if source.UseClientAuthorization() { | ||
| tokenStr, err = accessToken.ParseBearerToken() | ||
| if err != nil { | ||
| return nil, util.NewClientServerError("failed to parse access token", http.StatusInternalServerError, err) | ||
| } | ||
| } |
There was a problem hiding this comment.
When accessToken.ParseBearerToken() fails, it already returns a util.ToolboxError with a 401 Unauthorized status code. Wrapping it in a new util.NewClientServerError with http.StatusInternalServerError overrides this and returns a 500 Internal Server Error to the client, which is misleading for authentication failures. We should propagate the original util.ToolboxError if present, or default to http.StatusUnauthorized.
if source.UseClientAuthorization() {
tokenStr, err = accessToken.ParseBearerToken()
if err != nil {
if toolboxErr, ok := err.(util.ToolboxError); ok {
return nil, toolboxErr
}
return nil, util.NewClientServerError("failed to parse access token", http.StatusUnauthorized, err)
}
}b4d2439 to
dd2ddfd
Compare
dd2ddfd to
ec197d2
Compare
ec197d2 to
975ad26
Compare
975ad26 to
85a31a7
Compare
85a31a7 to
2957746
Compare
2957746 to
11cea03
Compare
Description
This PR introduces the
spanner-get-database-ddltool, allowing MCP agents to retrieve the complete schema definition (DDL statements) of a Cloud Spanner database.Changes:
DatabaseAdminClientlifecycle management and lazy on-demand initialization to SpannerSource.GetDatabaseDdl(ctx, tokenString)on SpannerSourceusing the Cloud Spanner Database Admin API.spanner-get-database-ddlwithReadOnlyAnnotations(readOnlyHint: true,destructiveHint: false).imports.goand added reference documentation inspanner-get-database-ddl.md.