feat(tools/looker): Enable Run Lookml Tests tool for Looker#2523
feat(tools/looker): Enable Run Lookml Tests tool for Looker#2523Anthrino wants to merge 2 commits into
Conversation
Summary of ChangesHello @Anthrino, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Looker integration by introducing a new tool designed to execute LookML tests directly from the toolbox. This capability streamlines the validation process for LookML changes, empowering developers to efficiently verify the integrity and correctness of their Looker projects without needing to switch environments. The addition improves the overall efficiency and reliability of LookML development workflows. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
The pull request successfully enables the Run Lookml Tests tool for Looker. The implementation follows the established patterns in the repository, including proper tool registration, configuration parsing, and SDK invocation. The addition of tests for configuration parsing ensures robustness. I have provided a few suggestions to improve error handling and code clarity in the tool's invocation logic.
| projectId, ok := mapParams["project_id"].(string) | ||
| if !ok { | ||
| return nil, fmt.Errorf("'project_id' must be a string, got %T", mapParams["project_id"]) | ||
| } |
There was a problem hiding this comment.
While ParseParams should have already validated that project_id is present and correctly typed, this manual check is good for defensive programming. However, if project_id is missing from mapParams, mapParams["project_id"] will be nil, and %T will produce <nil>. The error message is clear enough, but you might want to explicitly check for nil if you want a more specific message.
| var fileId *string | ||
| if val, ok := mapParams["file_id"].(string); ok && val != "" { | ||
| fileId = &val | ||
| } | ||
|
|
||
| var test *string | ||
| if val, ok := mapParams["test"].(string); ok && val != "" { | ||
| test = &val | ||
| } | ||
|
|
||
| var model *string | ||
| if val, ok := mapParams["model"].(string); ok && val != "" { | ||
| model = &val | ||
| } |
There was a problem hiding this comment.
The logic for handling optional parameters is correct. However, since these parameters are defined as TypeString in Initialize, ParseParams ensures they are strings if present. You can simplify the extraction by using a helper or just checking for the zero value if you trust the upstream validation.
| var fileId *string | |
| if val, ok := mapParams["file_id"].(string); ok && val != "" { | |
| fileId = &val | |
| } | |
| var test *string | |
| if val, ok := mapParams["test"].(string); ok && val != "" { | |
| test = &val | |
| } | |
| var model *string | |
| if val, ok := mapParams["model"].(string); ok && val != "" { | |
| model = &val | |
| } | |
| var fileId *string | |
| if val, _ := mapParams["file_id"].(string); val != "" { | |
| fileId = &val | |
| } | |
| var test *string | |
| if val, _ := mapParams["test"].(string); val != "" { | |
| test = &val | |
| } | |
| var model *string | |
| if val, _ := mapParams["model"].(string); val != "" { | |
| model = &val | |
| } |
Description
Adds a new MCP tool to run one or more LookML tests defined in a project for validation of changes via the toolbox.
Invokes the Run LookML Test Looker API (RunLookmlTest sdk method), which runs single or multiple tests by specifying the project_id, file_id, test & model parameters to narrow the scope as required.
PR Checklist
CONTRIBUTING.md
bug/issue
before writing your code! That way we can discuss the change, evaluate
designs, and agree on the general idea
!if this involve a breaking change🛠️ Fixes #2448