chore(ci): migrate mysql/mysql + proof of concept proxy helper #4040
Conversation
product-auto-label
bot
added
samples
![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 is a work-in-progress proof of concept for integrating Cloud SQL with MySQL. It includes changes to the CI configuration to enable testing, updates the package.json to include a proxy setup script, and adds a new script to set up the Cloud SQL proxy for testing.
Highlights
- CI Configuration: The pull request adds
cloud-sql/mysql/mysqlto thenodejs-dev.jsoncconfiguration file, enabling CI tests for this sample. It also removescloud-sql/mysql/mysqlfrom thenodejs-prod.jsoncconfiguration file, likely due to previous test failures. - Proxy Setup: A new shell script
test/proxy-setup.shis introduced to download and configure the Cloud SQL proxy. This script is executed before the system tests to establish a connection to the Cloud SQL instance. - Environment Variables: A
ci-setup.jsonfile is added to define environment variables required for the CI environment, including the Cloud SQL connection name, database name, user, and password (obtained from a secret).
Changelog
- .github/config/nodejs-dev.jsonc
- Added
cloud-sql/mysql/mysqlto the list of tested samples.
- Added
- .github/config/nodejs-prod.jsonc
- Removed
cloud-sql/mysql/mysqlfrom the list of tested samples, likely due to test failures.
- Removed
- cloud-sql/mysql/mysql/ci-setup.json
- Added a new file to define environment variables for the CI environment, including database connection details and credentials.
- cloud-sql/mysql/mysql/package.json
- Modified the
system-testscript to executetest/proxy-setup.shbefore running the tests.
- Modified the
- cloud-sql/mysql/mysql/test/proxy-setup.sh
- Added a new shell script to download, configure, and run the Cloud SQL proxy.
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.
Did you know?
MySQL is named after co-founder Michael 'Monty' Widenius's daughter, My.
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
This pull request introduces a proof of concept for connecting to Cloud SQL using MySQL. It includes necessary configurations and a script to set up the proxy. Overall, the changes seem reasonable, but there are a few points that need attention.
Summary of Findings
- Hardcoded version in proxy-setup.sh: The version of
cloud-sql-proxyis hardcoded intest/proxy-setup.sh. It's better to use a variable for the version to make it easier to update in the future. - Missing error handling in proxy-setup.sh: The
test/proxy-setup.shscript lacks error handling. It should check if thecurlcommand was successful and exit if it failed. - Unnecessary removal of cloud-sql/mysql/mysql from nodejs-prod.jsonc: The removal of
cloud-sql/mysql/mysqlfromnodejs-prod.jsoncseems counterintuitive, especially since the goal is to add support for Cloud SQL MySQL. It's marked as untested with an internal server error, but removing it entirely might not be the best approach.
Merge Readiness
The pull request introduces a proof of concept for Cloud SQL MySQL integration. However, the hardcoded version and lack of error handling in proxy-setup.sh should be addressed before merging. Additionally, the removal of cloud-sql/mysql/mysql from the production config should be reconsidered. I am unable to approve this pull request, and recommend that others review and approve this code before merging. At a minimum, the high severity issues should be addressed before merging.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
glasnt
changed the title
davidcavazos
left a comment
davidcavazos
left a comment
There was a problem hiding this comment.
Overall LGTM! Left some minor comments.
|
Quickchecked in #4054 that knex will cleanly migrate using this (did have to fix the usage to suit) |
…eCloudPlatform#4040) * wip: proof of concept cloud-sql mysql * chmod * add sleep to ensure proxy starts before use * use correct envvar * extra bit * debugging * instance_host is deleted by the server-unix test * proxy all tests * accepting gemini-code-assist code Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * oop * wip: tcp or unix * syntax * cause she looks like a flower * more shell logic * sh * continued proof of concept * call shutdown * format value * order? * debug: echo * hoisted by my own petard. * tmp file * -p * WIP: shared proxy setup * chmod * maybe? * ?? * bash * debug * debug * maybe * more bash * simplify usage * cleanup * actually use the new setting * remove old setting * attempt formatting * add mocha colour * at least use something * code review comments * syntax * syntax * fix usage --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
4 participants
Description
migrates cloud-sql/mysql/mysql to Custard CI
Introduces
sql-proxy.shhelper in the root(-ish) utils/ folder to assist in setting up either TCP or Unix socket connections through Cloud SQL Auth ProxyUsage docs on file, including
proxyscript target (rather than creating a new file in each sample), and theSOCKET=unixto use unix as opposed to a default tcp socket.Requires using a local folder for the socket connection, as /cloudsql is not accessible in github actions environment.
Should also fail and fail hard if tests fail, rather than silently failing.
Also turns out github actions has colors so let's make mocha do that (optional)