Skip to content

CloudSQL improvements: database flags, query insights, bump default version#4115

Merged
mr0re1 merged 1 commit into
GoogleCloudPlatform:developfrom
wiktorn:cloudsql_flags_query_insights
May 16, 2025
Merged

CloudSQL improvements: database flags, query insights, bump default version#4115
mr0re1 merged 1 commit into
GoogleCloudPlatform:developfrom
wiktorn:cloudsql_flags_query_insights

Conversation

@wiktorn

@wiktorn wiktorn commented May 13, 2025

Copy link
Copy Markdown
Contributor
  • add possibility to set database flags
  • allow query insights management
  • set default version to 8_0 to avoid extended support costs (see details here)

If deployment relies on default version being deployed, this change will automatically upgrade their database version (hence, I'm adding release-breaking-changes flag - though update should work in place)

Submission Checklist

NOTE: Community submissions can take up to 2 weeks to be reviewed.

Please take the following actions before submitting this pull request.

  • Fork your PR branch from the Toolkit "develop" branch (not main)
  • Test all changes with pre-commit in a local branch #
  • Confirm that "make tests" passes all tests
  • Add or modify unit tests to cover code changes
  • Ensure that unit test coverage remains above 80%
  • Update all applicable documentation
  • Follow Cluster Toolkit Contribution guidelines #

@wiktorn wiktorn requested review from a team and samskillman as code owners May 13, 2025 12:44

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @wiktorn, 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 focuses on enhancing the Cloud SQL module for Slurm federation. It introduces the ability to set database flags and manage query insights, and it updates the default MySQL version to 8_0. The changes span across the module's README, main Terraform configuration, and variables definition files, as well as a minor change to the controller configuration.

Highlights

  • Database Flags: The pull request introduces the ability to set database flags on the Cloud SQL instance. This is achieved through the database_flags variable, which is a map of string key-value pairs.
  • Query Insights Management: The pull request adds support for managing query insights on the Cloud SQL instance. This includes enabling/disabling query insights, setting the number of query plans per minute, query string length, and recording application tags and client addresses.
  • Default MySQL Version: The default MySQL version has been updated from 5_7 to 8_0 to avoid extended support costs.
  • Secret Manager Project: The pull request adds the project ID to the secret manager resource to ensure the secret is created in the correct project.

Changelog

Click here to see the changelog
  • community/modules/database/slurm-cloudsql-federation/README.md
    • Updated the example to use cloudsql as the id and network as the use value.
  • community/modules/database/slurm-cloudsql-federation/main.tf
    • Added a dynamic block for database_flags to allow setting database flags on the instance (lines 63-69).
    • Added an insights_config block to allow managing query insights (lines 71-77).
  • community/modules/database/slurm-cloudsql-federation/variables.tf
    • Updated the default database_version from MYSQL_5_7 to MYSQL_8_0 (line 27).
    • Added a database_flags variable to allow setting database flags (lines 40-44).
    • Added a query_insights variable to allow configuring query insights (lines 85-95).
  • community/modules/scheduler/schedmd-slurm-gcp-v6-controller/controller.tf
    • Added project = var.project_id to the google_secret_manager_secret resource to ensure the secret is created in the correct project (line 169).
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.


Flags flutter, insights gleam,
MySQL's version, a coder's dream.
Eight point oh sings,
As code takes wings,
In cloud's embrace, a flowing stream.

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.

@wiktorn wiktorn added the release-breaking-changes Prevents "smooth" re-deploy across versions label May 13, 2025

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

The pull request introduces several improvements to the CloudSQL module, including the ability to set database flags, manage query insights, and update the default database version. The changes seem well-structured and address the intended functionalities. However, there are a few areas that could benefit from further refinement and clarification.

Summary of Findings

  • Input Validation: Consider adding input validation for database_flags, query_plans_per_minute, and query_string_length in variables.tf to ensure they fall within acceptable ranges and formats.
  • Variable Descriptions: Enhance the descriptions for database_flags and the nested attributes of query_insights in variables.tf to improve clarity and usability.
  • Network ID Clarification: Clarify the purpose and expected format of the network variable in the README.md example.

Merge Readiness

The pull request introduces valuable enhancements to the CloudSQL module. Addressing the suggested improvements, particularly regarding input validation and variable descriptions, will further enhance the module's robustness and usability. I am unable to directly approve this pull request, and recommend that others review and approve this code before merging. At a minimum, the above comments should be addressed before merging.

Comment thread community/modules/database/slurm-cloudsql-federation/main.tf
Comment thread community/modules/database/slurm-cloudsql-federation/main.tf
* add possibility to set database flags
* allow query insights management
* set default version to 8_0 to avoid extended support costs
@wiktorn wiktorn force-pushed the cloudsql_flags_query_insights branch from 9175c15 to 88f10fc Compare May 13, 2025 12:59
@mr0re1

mr0re1 commented May 15, 2025

Copy link
Copy Markdown
Collaborator

/gcbrun

@mr0re1 mr0re1 self-assigned this May 15, 2025
@mr0re1 mr0re1 enabled auto-merge May 15, 2025 21:24
@abbas1902 abbas1902 assigned wiktorn and unassigned mr0re1 May 16, 2025
@mr0re1 mr0re1 merged commit 85229dc into GoogleCloudPlatform:develop May 16, 2025
11 of 65 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-breaking-changes Prevents "smooth" re-deploy across versions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants