feat(tool/looker): add conversation CRUD tools#3032
feat(tool/looker): add conversation CRUD tools#3032hiracky16 wants to merge 1 commit intogoogleapis:mainfrom
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces five new Looker tools for managing conversation sessions: looker-create-conversation, looker-delete-conversation, looker-get-conversation, looker-list-conversations, and looker-update-conversation. The implementation includes the necessary Go logic, unit tests, and documentation for each tool. Several issues were identified regarding logging practices, specifically the use of ErrorContext for informational messages and the lack of structured key-value pairs. Additionally, the documentation titles need to be updated to follow the repository's naming convention, and mutating tools like 'create' and 'update' should include the DestructiveHint annotation for consistency. Finally, the imports in cmd/internal/imports.go should be reordered alphabetically.
| if err != nil { | ||
| return nil, util.ProcessGeneralError(err) | ||
| } | ||
| logger.ErrorContext(ctx, "Got response %v", resp) |
There was a problem hiding this comment.
Use InfoContext instead of ErrorContext for successful response logging, and provide arguments as key-value pairs.
| logger.ErrorContext(ctx, "Got response %v", resp) | |
| logger.InfoContext(ctx, "Got response", "response", resp) |
| req.AgentId = &agentId | ||
| } | ||
|
|
||
| logger.ErrorContext(ctx, "Making SearchConversations request: %v", req) |
There was a problem hiding this comment.
The logging level ErrorContext is inappropriate for an informational message. Please use InfoContext and provide arguments as key-value pairs.
| logger.ErrorContext(ctx, "Making SearchConversations request: %v", req) | |
| logger.InfoContext(ctx, "Making SearchConversations request", "request", req) |
| if err != nil { | ||
| return nil, util.ProcessGeneralError(err) | ||
| } | ||
| logger.ErrorContext(ctx, "Got response %v", resp) |
There was a problem hiding this comment.
Use InfoContext instead of ErrorContext for successful response logging, and provide arguments as key-value pairs.
| logger.ErrorContext(ctx, "Got response %v", resp) | |
| logger.InfoContext(ctx, "Got response", "response", resp) |
| Name: &name, | ||
| } | ||
|
|
||
| logger.ErrorContext(ctx, "Making UpdateConversation request for conversation ID: %s", conversationId) |
There was a problem hiding this comment.
The logging level ErrorContext is inappropriate for an informational message. Please use InfoContext and provide arguments as key-value pairs.
| logger.ErrorContext(ctx, "Making UpdateConversation request for conversation ID: %s", conversationId) | |
| logger.InfoContext(ctx, "Making UpdateConversation request", "conversation_id", conversationId) |
| if err != nil { | ||
| return nil, util.ProcessGeneralError(err) | ||
| } | ||
| logger.ErrorContext(ctx, "Got response %v", resp) |
There was a problem hiding this comment.
Use InfoContext instead of ErrorContext for successful response logging, and provide arguments as key-value pairs.
| logger.ErrorContext(ctx, "Got response %v", resp) | |
| logger.InfoContext(ctx, "Got response", "response", resp) |
| @@ -0,0 +1,46 @@ | |||
| --- | |||
| title: "looker-update-conversation" | |||
There was a problem hiding this comment.
The title in the YAML frontmatter should include the ' Tool' suffix to adhere to the repository's documentation standards.
| title: "looker-update-conversation" | |
| title: "looker-update-conversation Tool" |
References
- For tool documentation files, the title in the YAML frontmatter should use kebab-case for the tool name, followed by ' Tool'.
| @@ -0,0 +1,49 @@ | |||
| --- | |||
| title: "looker-list-conversations" | |||
There was a problem hiding this comment.
The title in the YAML frontmatter should include the ' Tool' suffix to adhere to the repository's documentation standards.
| title: "looker-list-conversations" | |
| title: "looker-list-conversations Tool" |
References
- For tool documentation files, the title in the YAML frontmatter should use kebab-case for the tool name, followed by ' Tool'.
| readOnlyHint := false | ||
| annotations := &tools.ToolAnnotations{ | ||
| ReadOnlyHint: &readOnlyHint, | ||
| } |
There was a problem hiding this comment.
This tool performs an 'Update' operation which modifies state. Please include the DestructiveHint annotation to ensure consistency with the PR's stated goals and other mutating tools.
| readOnlyHint := false | |
| annotations := &tools.ToolAnnotations{ | |
| ReadOnlyHint: &readOnlyHint, | |
| } | |
| readOnlyHint := false | |
| destructiveHint := true | |
| annotations := &tools.ToolAnnotations{ | |
| ReadOnlyHint: &readOnlyHint, | |
| DestructiveHint: &destructiveHint, | |
| } |
| @@ -0,0 +1,45 @@ | |||
| --- | |||
| title: "looker-get-conversation" | |||
There was a problem hiding this comment.
The title in the YAML frontmatter should include the ' Tool' suffix to adhere to the repository's documentation standards.
| title: "looker-get-conversation" | |
| title: "looker-get-conversation Tool" |
References
- For tool documentation files, the title in the YAML frontmatter should use kebab-case for the tool name, followed by ' Tool'.
| @@ -0,0 +1,44 @@ | |||
| --- | |||
| title: "looker-delete-conversation" | |||
There was a problem hiding this comment.
The title in the YAML frontmatter should include the ' Tool' suffix to adhere to the repository's documentation standards.
| title: "looker-delete-conversation" | |
| title: "looker-delete-conversation Tool" |
References
- For tool documentation files, the title in the YAML frontmatter should use kebab-case for the tool name, followed by ' Tool'.
2 participants
1. Description
This PR implements the Phase 1 of Looker's Conversational Analytics API integration by adding five tools for Conversation CRUD management. These tools follow the established patterns and the strict security/annotation guidelines established in PR #2830.
New Tools Added:
looker-create-conversation: Starts a new session with an AI agent.looker-list-conversations: Searches and lists existing conversation sessions.looker-get-conversation: Retrieves detailed metadata for a specific session.looker-update-conversation: Modifies conversation attributes (e.g., name).looker-delete-conversation: Deletes a conversation session (marked withDestructiveHint).Key Highlights:
ReadOnlyHintandDestructiveHintin theInitializemethod to ensure safe LLM interaction.docs/en/integrations/looker/tools/with correct H2 ordering and required shortcodes.2. PR Checklist
!if this involves a breaking change (N/A, additive only)3. Issue Reference
Related to the work on Looker Conversational Analytics integration. 🦕