Change to using a unix socket for mysql

[micheas]

Finishing the incomplete fix before

[gemini-code-assist]

Summary of Changes

Hello @micheas, I'm Gemini Code Assist¹! 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 finalizes an incomplete fix by modifying the database connection configuration within the config.cloudrun.yaml file. The core change involves transitioning the MySQL connection from a traditional TCP/IP setup to utilizing a Unix socket, specifically for Google Cloud SQL. This adjustment streamlines the connection process and is likely intended to optimize performance or security within the Cloud Run environment.

Highlights

• Database Connection Method: The MySQL database connection method has been updated from a standard TCP/IP connection to a Unix socket connection.
• Host Configuration: The db.host parameter in config.cloudrun.yaml now specifies a Unix socket path (/cloudsql/${DB_CONNECTION_NAME}) instead of a network hostname.
• Port Removal: The explicit port: 3306 configuration has been removed, as it is not applicable when connecting via a Unix socket.
• Connection String Update: The database connection string format (db.str) has been adjusted to reflect the removal of the port component.

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 by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

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 pull request 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 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

1. 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. ↩

[gemini-code-assist]

Code Review

This pull request aims to configure the application to connect to a MySQL database using a Unix socket, which is appropriate for a Cloud Run environment. However, the current implementation for constructing the database connection string is incorrect for the pymysql driver. I've identified a critical issue and provided comments with code suggestions to ensure the connection can be established successfully. The key is to use the unix_socket query parameter in the connection URL instead of placing the socket path in the host field.

[micheas]

/gemini review

[gemini-code-assist]

Code Review

The change to use a Unix socket for the MySQL connection in the Cloud Run configuration is correct and follows the recommended practice for connecting to Cloud SQL. However, I've identified a significant underlying issue in how the database connection string is constructed. The current method is not safe and can lead to connection failures if credentials contain special characters. I've provided a detailed comment with a recommendation for a more robust implementation.

[micheas]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors the database connection logic to support MySQL connections via a Unix socket, which is a good improvement. The implementation correctly removes the hardcoded connection string format from configuration files and builds the SQLAlchemy URL programmatically. However, I've found a critical bug in the new logic due to typos that will prevent the application from working as intended. My review includes a fix for this issue.

[micheas]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors the database connection logic to programmatically build the connection URL, which is a great improvement over string formatting. This change also successfully adds support for connecting to MySQL via a Unix socket, as intended. The changes in the configuration files are consistent with this new approach. I've added one suggestion in src/oncall/db.py to also handle the echo parameter from the configuration, which seems to have been ignored previously. Overall, this is a solid improvement.

[micheas]

/gemini review

[gemini-code-assist]

Code Review

This pull request refactors the database connection logic to support MySQL connections via a Unix socket, which is particularly useful for environments like Google Cloud Run. The change replaces manual string formatting of the connection URL with SQLAlchemy's URL.create method, making the process more robust and secure. This refactoring also correctly enables the echo parameter for create_engine, which was previously ignored. The changes look good and improve the codebase. I have one suggestion regarding the placement of the echo configuration parameter for better structural clarity in the long run.

[gemini-code-assist]

This correctly passes the echo parameter to create_engine. Previously, this setting was in conn_kwargs but was not used. While this fixes the issue, semantically, echo is an engine parameter, not a connection parameter. For better configuration structure, it would be ideal to move the echo key from db.conn.kwargs to db.kwargs in the YAML configuration files. If you make that change, you can remove this line, as echo would then be passed via **config['kwargs'].