-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix: something need to be fixed #2276
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
base: main
Are you sure you want to change the base?
Conversation
Summary of ChangesHello @rahulpinto19, 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 introduces a fix primarily focused on enhancing the reliability and thoroughness of PostgreSQL test environment cleanup. It streamlines the process by adopting a more aggressive schema reset mechanism and removes an apparently unused or deprecated tool configuration. These changes aim to prevent test flakiness and ensure consistent database states across test runs. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request appears to remove the postgres-list-stored-procedure tool and refactors the CleanupPostgresTables test helper function. The changes to remove the tool seem correct based on the provided diff. However, the new implementation of CleanupPostgresTables can be improved. Specifically, it leaves behind a large block of commented-out code that should be removed, and the new logic for cleaning the database schema could be made more robust by executing SQL statements individually for better error handling.
tests/common.go
Outdated
| // query := ` | ||
| // SELECT table_name FROM information_schema.tables | ||
| // WHERE table_schema = 'public' AND table_type = 'BASE TABLE';` | ||
|
|
||
| // rows, err := pool.Query(ctx, query) | ||
| // if err != nil { | ||
| // t.Fatalf("Failed to query for all tables in 'public' schema: %v", err) | ||
| // } | ||
| // defer rows.Close() | ||
|
|
||
| // var tablesToDrop []string | ||
| // for rows.Next() { | ||
| // var tableName string | ||
| // if err := rows.Scan(&tableName); err != nil { | ||
| // t.Errorf("Failed to scan table name: %v", err) | ||
| // continue | ||
| // } | ||
| // tablesToDrop = append(tablesToDrop, fmt.Sprintf("public.%q", tableName)) | ||
| // } | ||
|
|
||
| // if len(tablesToDrop) == 0 { | ||
| // return | ||
| // } | ||
|
|
||
| // dropQuery := fmt.Sprintf("DROP TABLE IF EXISTS %s CASCADE;", strings.Join(tablesToDrop, ", ")) | ||
|
|
||
| // if _, err := pool.Exec(ctx, dropQuery); err != nil { | ||
| // t.Fatalf("Failed to drop all tables in 'public' schema: %v", err) | ||
| // } |
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.
tests/common.go
Outdated
| // 1. Drop the entire public schema (this kills tables, views, types, etc.) | ||
| dropSchema := "DROP SCHEMA public CASCADE;" | ||
| // 2. Recreate the empty public schema | ||
| createSchema := "CREATE SCHEMA public;" | ||
| // 3. Grant permissions back (Postgres default) | ||
| grantPublic := "GRANT ALL ON SCHEMA public TO public;" | ||
|
|
||
| _, err := pool.Exec(ctx, dropSchema + createSchema + grantPublic) | ||
| if err != nil { | ||
| t.Fatalf("Failed to nuclear-wipe the public schema: %v", err) | ||
| } |
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.
While pgx can execute multiple statements in a single Exec call, it's better practice to execute each statement separately. This improves error handling by pinpointing which statement failed and makes the code easier to read and debug.
// 1. Drop the entire public schema (this kills tables, views, types, etc.)
if _, err := pool.Exec(ctx, "DROP SCHEMA public CASCADE;"); err != nil {
t.Fatalf("Failed to drop public schema: %v", err)
}
// 2. Recreate the empty public schema
if _, err := pool.Exec(ctx, "CREATE SCHEMA public;"); err != nil {
t.Fatalf("Failed to create public schema: %v", err)
}
// 3. Grant permissions back (Postgres default)
if _, err := pool.Exec(ctx, "GRANT ALL ON SCHEMA public TO public;"); err != nil {
t.Fatalf("Failed to grant permissions on public schema: %v", err)
}d2be5f5 to
1bfe394
Compare
## Description Adds a section in the navbar that links to the toolbox medium blog: <img width="492" height="822" alt="87F2yTQdcbpMHs3" src="https://github.com/user-attachments/assets/74d8b552-1e8f-449c-8b09-4f86218d2817" /> ## PR Checklist > Thank you for opening a Pull Request! Before submitting your PR, there are a > few things you can do to make sure it goes smoothly: - [ ] Make sure you reviewed [CONTRIBUTING.md](https://github.com/googleapis/genai-toolbox/blob/main/CONTRIBUTING.md) - [ ] Make sure to open an issue as a [bug/issue](https://github.com/googleapis/genai-toolbox/issues/new/choose) before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea - [ ] Ensure the tests and linter pass - [ ] Code coverage does not decrease (if any source code was changed) - [ ] Appropriate docs were updated (if necessary) - [ ] Make sure to add `!` if this involve a breaking change 🛠️ Fixes #<issue_number_goes_here> --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Embedding Models were only loaded in hot reload because it was not initialized properly.
## Description Previously added `allowed-origins` (for CORs) is not sufficient for preventing DNS rebinding attacks. We'll have to check host headers. To test, run Toolbox with the following: ``` go run . --allowed-hosts=127.0.0.1:5000 ``` Test with the following: ``` // curl successfully curl -H "Host: 127.0.0.1:5000" http://127.0.0.1:5000 // will show Invalid Host Header error curl -H "Host: attacker:5000" http://127.0.0.1:5000 ``` ## PR Checklist > Thank you for opening a Pull Request! Before submitting your PR, there are a > few things you can do to make sure it goes smoothly: - [ ] Make sure you reviewed [CONTRIBUTING.md](https://github.com/googleapis/genai-toolbox/blob/main/CONTRIBUTING.md) - [ ] Make sure to open an issue as a [bug/issue](https://github.com/googleapis/genai-toolbox/issues/new/choose) before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea - [ ] Ensure the tests and linter pass - [ ] Code coverage does not decrease (if any source code was changed) - [ ] Appropriate docs were updated (if necessary) - [ ] Make sure to add `!` if this involve a breaking change 🛠️ Fixes #<issue_number_goes_here>
## Description Add default value to manifest (for both native endpoint and mcp endpoint). ## PR Checklist > Thank you for opening a Pull Request! Before submitting your PR, there are a > few things you can do to make sure it goes smoothly: - [x] Make sure you reviewed [CONTRIBUTING.md](https://github.com/googleapis/genai-toolbox/blob/main/CONTRIBUTING.md) - [x] Make sure to open an issue as a [bug/issue](https://github.com/googleapis/genai-toolbox/issues/new/choose) before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea - [x] Ensure the tests and linter pass - [x] Code coverage does not decrease (if any source code was changed) - [x] Appropriate docs were updated (if necessary) - [x] Make sure to add `!` if this involve a breaking change 🛠️ Fixes #1602
…2265) ## Description PSV should only be required when when it is needed. Currently, we require psv even whenever user uses AlloyDB AI NL tool. This is due to the statement that we use to execute nl query. This PR modified the statement query to only utilize `param_names` and `param_values` when needed. Manually tested with a db that does not have psv installed. 🛠️ Fixes #1970
Update hugo version for release v0.25.0
🤖 I have created a release *beep* *boop* --- ## [0.25.0](v0.24.0...v0.25.0) (2026-01-08) ### Features * Add `embeddingModel` support ([#2121](#2121)) ([9c62f31](9c62f31)) * Add `allowed-hosts` flag ([#2254](#2254)) ([17b41f6](17b41f6)) * Add parameter default value to manifest ([#2264](#2264)) ([9d1feca](9d1feca)) * **snowflake:** Add Snowflake Source and Tools ([#858](#858)) ([b706b5b](b706b5b)) * **prebuilt/cloud-sql-mysql:** Update CSQL MySQL prebuilt tools to use IAM ([#2202](#2202)) ([731a32e](731a32e)) * **sources/bigquery:** Make credentials scope configurable ([#2210](#2210)) ([a450600](a450600)) * **sources/trino:** Add ssl verification options and fix docs example ([#2155](#2155)) ([4a4cf1e](4a4cf1e)) * **tools/looker:** Add ability to set destination folder with `make_look` and `make_dashboard`. ([#2245](#2245)) ([eb79339](eb79339)) * **tools/postgressql:** Add tool to list store procedure ([#2156](#2156)) ([cf0fc51](cf0fc51)) * **tools/postgressql:** Add Parameter `embeddedBy` config support ([#2151](#2151)) ([17b70cc](17b70cc)) ### Bug Fixes * **server:** Add `embeddingModel` config initialization ([#2281](#2281)) ([a779975](a779975)) * **sources/cloudgda:** Add import for cloudgda source ([#2217](#2217)) ([7daa411](7daa411)) * **tools/alloydb-wait-for-operation:** Fix connection message generation ([#2228](#2228)) ([7053fbb](7053fbb)) * **tools/alloydbainl:** Only add psv when NL Config Param is defined ([#2265](#2265)) ([ef8f3b0](ef8f3b0)) * **tools/looker:** Looker client OAuth nil pointer error ([#2231](#2231)) ([268700b](268700b)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Yuan Teoh <[email protected]>
## Description update docs to reflect triaging workflow and SLO ## PR Checklist > Thank you for opening a Pull Request! Before submitting your PR, there are a > few things you can do to make sure it goes smoothly: - [x] Make sure you reviewed [CONTRIBUTING.md](https://github.com/googleapis/genai-toolbox/blob/main/CONTRIBUTING.md) - [x] Make sure to open an issue as a [bug/issue](https://github.com/googleapis/genai-toolbox/issues/new/choose) before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea - [x] Ensure the tests and linter pass - [x] Code coverage does not decrease (if any source code was changed) - [x] Appropriate docs were updated (if necessary) - [x] Make sure to add `!` if this involve a breaking change 🛠️ Fixes #<issue_number_goes_here>
…2262) This change allows the agent developer to control the maxium number of rows returned from tools running BigQuery SQL query. Using this feature the agent developer could limit how large output is presented to LLM in an agentic user journey. ## Description > Should include a concise description of the changes (bug or feature), it's > impact, along with a summary of the solution ## PR Checklist > Thank you for opening a Pull Request! Before submitting your PR, there are a > few things you can do to make sure it goes smoothly: - [x] Make sure you reviewed [CONTRIBUTING.md](https://github.com/googleapis/genai-toolbox/blob/main/CONTRIBUTING.md) - [ ] Make sure to open an issue #2261 before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea - [x] Ensure the tests and linter pass - [x] Code coverage does not decrease (if any source code was changed) - [x] Appropriate docs were updated (if necessary) - [ ] Make sure to add `!` if this involve a breaking change 🛠️ Fixes #2261 2261
Description
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 #<issue_number_goes_here>