chore(cloud-sql): migrate mysql, postgres samples to new CI #4055
Conversation
product-auto-label
bot
added
api: cloudsql
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @glasnt, 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!
Summary of Changes
This pull request migrates the MySQL and PostgreSQL samples to the new CI (Continuous Integration) setup. This involves updating the CI configuration files and modifying the test scripts to align with the new CI environment. Specifically, the changes include adding new CI setup files for MySQL and PostgreSQL, updating the nodejs-dev.jsonc file to include the new samples, and removing the samples from the nodejs-prod.jsonc file. Additionally, the index.js file for MySQL is modified to use the connect-connector-auto-iam-authn.js file, and the package.json file for PostgreSQL is updated to include a proxy script and modify the test script.
Highlights
- CI Migration: The primary goal is to migrate the MySQL and PostgreSQL samples to the new CI environment.
- Configuration Updates: The
.github/config/nodejs-dev.jsoncand.github/config/nodejs-prod.jsoncfiles are updated to reflect the new CI setup. - Test Script Modification: The test script in
cloud-sql/postgres/knex/package.jsonis modified to include a proxy command. - New CI Setup Files: New
ci-setup.jsonfiles are added for both MySQL and PostgreSQL samples.
Changelog
Click here to see the changelog
- .github/config/nodejs-dev.jsonc
- Added
cloud-sql/mysql/mysql2andcloud-sql/postgres/knexto the list of samples to be tested in the development CI.
- Added
- .github/config/nodejs-prod.jsonc
- Removed
cloud-sql/mysql/mysql2andcloud-sql/postgres/knexfrom the list of samples to be tested in the production CI. This likely means they are now passing tests and no longer need to be excluded.
- Removed
- cloud-sql/mysql/mysql2/ci-setup.json
- Added a new file
ci-setup.jsonto configure the CI environment for the MySQL sample using mysql2. - The file defines environment variables such as
INSTANCE_HOST,INSTANCE_CONNECTION_NAME,UNIX_SOCKET_DIR,CLOUD_SQL_CONNECTION_NAME,INSTANCE_UNIX_SOCKET,DB_NAME, andDB_USER. - It also defines a secret for
DB_PASS.
- Added a new file
- cloud-sql/mysql/mysql2/index.js
- Modified the
index.jsfile to useconnect-connector-auto-iam-authn.jsinstead ofconnect-connector-with-iam-authn.js. This likely simplifies the IAM authentication process.
- Modified the
- cloud-sql/postgres/knex/ci-setup.json
- Added a new file
ci-setup.jsonto configure the CI environment for the PostgreSQL sample using knex. - The file defines environment variables such as
INSTANCE_HOST,INSTANCE_CONNECTION_NAME,UNIX_SOCKET_DIR,CLOUD_SQL_CONNECTION_NAME,INSTANCE_UNIX_SOCKET,DB_NAME, andDB_USER. - It also defines a secret for
DB_PASS.
- Added a new file
- cloud-sql/postgres/knex/package.json
- Added a
proxyscript that executessql-proxy.sh. - Modified the
testscript to run theproxyscript before executing the tests. This ensures that the Cloud SQL proxy is running before the tests start.
- Added a
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and 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 to provide feedback.
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.
Trivia time!
What is Knex.js and what database systems does it support?
Click here for the answer
Knex.js is a "batteries included" SQL query builder for Javascript. It supports PostgreSQL, MySQL, SQLite3, MariaDB, Oracle, and MSSQL.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
The pull request migrates MySQL and PostgreSQL samples to a new CI. The changes involve updating configuration files and modifying code to use the Cloud SQL Node.js Connector. Overall, the changes seem reasonable, but there are a few areas that could be improved.
Summary of Findings
- Missing Error Handling: The
proxyscript inpackage.jsonfor the PostgreSQL sample does not include error handling. If the script fails, the tests will not run correctly, leading to false positives or failures. - Inconsistent Naming: In
cloud-sql/mysql/mysql2/index.js, thecreateConnectorIAMAuthnPoolfunction is being assigned a file path that containsautoin the name, but incloud-sql/postgres/knex/index.jsit is not. This could lead to confusion.
Merge Readiness
The pull request is almost ready for merging, but there are a few issues that need to be addressed. Specifically, the missing error handling in the proxy script for the PostgreSQL sample should be fixed. Additionally, the naming inconsistency in the createConnectorIAMAuthnPool function assignment should be addressed for clarity. I am unable to approve this pull request, and recommend that another reviewer does so after these changes are made.
|
The MySQL2 sample was failing for reasons unrelated to the migration:
|
|
mysql/mysql2 was introduced in #3264 but that commit never contributed a |
No region tags are edited in this PR.This comment is generated by snippet-bot.
|
glasnt
dismissed
gemini-code-assist[bot]’s stale review
bot can't resolve requested changes
jackwotherspoon
left a comment
jackwotherspoon
left a comment
There was a problem hiding this comment.
Thanks @glasnt 👍
…oudPlatform#4055) * chore(cloud-sql): migrate mysql, postgres samples to new CI * rename to match postgres version * acquireTimeout warns in mysql2, remove * mysql2 query returns [results, fields], so only return fields * remove unused test scripts * revert bot-suggested rename: would cause excessive region tag change
…oudPlatform#4055) * chore(cloud-sql): migrate mysql, postgres samples to new CI * rename to match postgres version * acquireTimeout warns in mysql2, remove * mysql2 query returns [results, fields], so only return fields * remove unused test scripts * revert bot-suggested rename: would cause excessive region tag change
…oudPlatform#4055) * chore(cloud-sql): migrate mysql, postgres samples to new CI * rename to match postgres version * acquireTimeout warns in mysql2, remove * mysql2 query returns [results, fields], so only return fields * remove unused test scripts * revert bot-suggested rename: would cause excessive region tag change
…oudPlatform#4055) * chore(cloud-sql): migrate mysql, postgres samples to new CI * rename to match postgres version * acquireTimeout warns in mysql2, remove * mysql2 query returns [results, fields], so only return fields * remove unused test scripts * revert bot-suggested rename: would cause excessive region tag change
* Added CRUD code snippets with codeowners file * Solved linting errors * Added sanitization code snippets along with test cases * Added header comment * Removed hardcoded value from test file * Removed crud related files * Addressed gemini bot comments * Updated prompts, tests and log statements for sanitization APIs * chore(cloud-sql): migrate mysql, postgres samples to new CI (#4055) * chore(cloud-sql): migrate mysql, postgres samples to new CI * rename to match postgres version * acquireTimeout warns in mysql2, remove * mysql2 query returns [results, fields], so only return fields * remove unused test scripts * revert bot-suggested rename: would cause excessive region tag change * chore(compute): Migrate region tag (#4047) * chore(compute): Migrate region tag * chore(compute): Rename region tag * chore(compute): Renove older region tag (#4062) overriding failure as this is jsut removing the region tag. * feat: support testing on forked repos (#4057) * feat: support testing on forked repos * add pull_request to test on PR * remove prod suffix * decode json on matrix paths * add experimental to check names * add experimental to job names * run test only if paths is not empty * test explicitly against empty array * move experimental as a prefix * success check if no paths are found * always set check to done * remove pull_request trigger * rearrange if check * add more comments * fix: add id-token to workflows (#4063) * feat: support testing on forked repos * add pull_request to test on PR * remove prod suffix * decode json on matrix paths * add experimental to check names * add experimental to job names * run test only if paths is not empty * test explicitly against empty array * move experimental as a prefix * success check if no paths are found * always set check to done * remove pull_request trigger * rearrange if check * add more comments * fix: add id-token to workflows * chore: update the minimum language based on maintenance mode * chore(deps): pin dependencies (#4035) * Added region tags to snippets * Updated region tags for code snippets based on errors * Removed extra csam assertions in sanitization tests * Addressed review comments * Updated test cases for sanitization snippets, added test for rai filter * Updated tests as per review comments * updated commented variables sample values in code snippets * Addressed review comments and updated tests accordingly * removed-todo-developer-comments * add-new-line-eslintrc * fix-linting-warnings-and-errors * fixed-tests * revert-security-center-change * revert-security-center-change * revert-security-center-change * fix-minor-lint --------- Co-authored-by: Eric Schmidt <[email protected]> Co-authored-by: Harsh Nasit <[email protected]> Co-authored-by: harshnasitcrest <[email protected]> Co-authored-by: David Cavazos <[email protected]>
3 participants
Description
Fixes #
Note: Before submitting a pull request, please open an issue for discussion if you are not associated with Google.
Checklist
npm test(see Testing)npm run lint(see Style)GoogleCloudPlatform/nodejs-docs-samples. Not a fork.