Skip to content
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

Add a diffSupressFunc() for template_gcs_path. #13207

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

harrisonlimh
Copy link
Contributor

@harrisonlimh harrisonlimh commented Feb 26, 2025

Fixes hashicorp/terraform-provider-google#21567

Add a diffSupressFunc() for template_gcs_path.

Release Note Template for Downstream PRs (will be copied)

See Write release notes for guidance.

dataflow: fixed a permadiff on `template_gcs_path` in `google_dataflow_job` resource

Copy link

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

Choose a reason for hiding this comment

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

Hello @harrisonlimh, 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

I am currently reviewing this pull request. In the meantime, I am providing a summary of the changes for you and other reviewers to quickly understand the intent and content of this pull request. This pull request adds a DiffSuppressFunc to the template_gcs_path field in the ResourceDataflowJob resource. This function will suppress diffs for this field, which is useful when the value of the field is not important for the resource to function correctly.

Highlights

  • Diff Suppression: The pull request introduces DiffSuppressFunc: tpgresource.AlwaysDiffSuppress for the template_gcs_path attribute.
  • Resource Definition: The change affects the template_gcs_path field within the ResourceDataflowJob resource definition.

Changelog

  • mmv1/third_party/terraform/services/dataflow/resource_dataflow_job.go.tmpl
    • Added DiffSuppressFunc: tpgresource.AlwaysDiffSuppress to the template_gcs_path field to suppress diffs.
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 the purpose of a DiffSuppressFunc in Terraform?

Click here for the answer
A DiffSuppressFunc in Terraform is used to suppress diffs for a particular attribute. This is useful when the value of the attribute is not important for the resource to function correctly, or when the value is automatically generated and changes frequently.

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.

Copy link

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

Choose a reason for hiding this comment

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

Code Review

The code change adds a DiffSuppressFunc to the template_gcs_path field in the Dataflow job resource schema. This is a good addition for cases where the GCS path might change without affecting the underlying resource. The implementation is straightforward and well-placed.

Summary of Findings

  • Placement of DiffSuppressFunc: The DiffSuppressFunc is placed before the Description field. While this works, it's more conventional to place it after the Type and Required fields, but before the Description field for better readability.

Assessment

The code change adds a DiffSuppressFunc to the template_gcs_path field in the Dataflow job resource schema. This is a useful addition for cases where the GCS path might change without affecting the underlying resource. The implementation is straightforward and well-placed. I would recommend addressing these comments before requesting a review from someone else, but feel free to request another review from Gemini via /gemini review when you have addressed these comments and I'll take another look! Remember that in any circumstance, users should have others review and approve this code before merging.

Copy link

github-actions bot commented Feb 26, 2025

Hello! I am a robot. Tests will require approval from a repository maintainer to run.

@shuyama1, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look.

You can help make sure that review is quick by doing a self-review and by running impacted tests locally.

@github-actions github-actions bot requested a review from hao-nan-li February 26, 2025 22:29
@modular-magician modular-magician added the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Feb 26, 2025
@shuyama1 shuyama1 requested review from shuyama1 and removed request for hao-nan-li February 26, 2025 22:31
@@ -87,6 +87,7 @@ func ResourceDataflowJob() *schema.Resource {
"template_gcs_path": {
Type: schema.TypeString,
Required: true,
DiffSuppressFunc: tpgresource.AlwaysDiffSuppress,
Copy link
Member

Choose a reason for hiding this comment

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

We'd want to only suppress this specific diff - bucket name adds region suffix, instead of all diffs, as we still need to detect changes for this field.

I don't think we currently have a diff suppress function that can handle this case specifically. You'll need to write a new function yourself, likely named resourceDataflowJobTemplateGcsPathDiffSuppress.

See resourceDataflowJobLabelDiffSuppress function as an example.

old is the value read from the API, like "gs://dataflow-templates-us-central1/latest/Word_Count"
new is what in the config, like "gs://dataflow-templates/latest/Word_Count"
region value could likely be retrieved by d.Get("region")

Please let me know if you have any other questions, thank you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you! Done.

@github-actions github-actions bot requested a review from shuyama1 February 27, 2025 23:09
@modular-magician modular-magician added service/dataflow and removed awaiting-approval Pull requests that need reviewer's approval to run presubmit tests labels Feb 27, 2025
@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 2 files changed, 55 insertions(+), 3 deletions(-))
google-beta provider: Diff ( 2 files changed, 55 insertions(+), 3 deletions(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 0
Passed tests: 0
Skipped tests: 0
Affected tests: 0

Click here to see the affected service packages
  • dataflow

🔴 Errors occurred during REPLAYING mode. Please fix them to complete your PR.

View the build log

@modular-magician modular-magician added the awaiting-approval Pull requests that need reviewer's approval to run presubmit tests label Feb 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-approval Pull requests that need reviewer's approval to run presubmit tests service/dataflow
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failing test(s): TestAccDataflowJob_basic
3 participants