Fix dataflow flextemplate job labels #6559

AarshDhokai · 2022-09-16T17:23:40Z

fixes the issue : Reintroduce support for Dataflow labels in FlexTemplate

If this PR is for Terraform, I acknowledge that I have:

Searched through the issue tracker for an open issue that this either resolves or contributes to, commented on it to claim it, and written "fixes {url}" or "part of {url}" in this PR description. If there were no relevant open issues, I opened one and commented that I would like to work on it (not necessary for very small changes).
Generated Terraform, and ran make test and make lint to ensure it passes unit and linter tests.
Ensured that all new fields I added that can be set by a user appear in at least one example (for generated resources) or third_party test (for handwritten resources or update tests).
Ran relevant acceptance tests (If the acceptance tests do not yet pass or you are unable to run them, please let your reviewer know).
Read the Release Notes Guide before writing my release note below.

Release Note Template for Downstream PRs (will be copied)

dataflow : added support of `labels` to resource `google_dataflow_flextemplate_job` (beta)

modular-magician · 2022-09-16T17:23:56Z

Oops! It looks like you're using an unknown release-note type in your changelog entries:

REPLACEME

Please only use the types listed in https://github.com/GoogleCloudPlatform/magic-modules/blob/master/.ci/RELEASE_NOTES_GUIDE.md.

modular-magician · 2022-09-16T17:24:40Z

Hello! I am a robot who works on Magic Modules PRs.

I've detected that you're a community contributor. @melinath, a repository maintainer, has been assigned to assist you and help review your changes.

❓ First time contributing? Click here for more details

Your assigned reviewer will help review your code by:

Ensuring it's backwards compatible, covers common error cases, etc.
Summarizing the change into a user-facing changelog note.
Passes tests, either our "VCR" suite, a set of presubmit tests, or with manual test runs.

You can help make sure that review is quick by running local tests and ensuring they're passing in between each push you make to your PR's branch. Also, try to leave a comment with each push you make, as pushes generally don't generate emails.

If your reviewer doesn't get back to you within a week after your most recent change, please feel free to leave a comment on the issue asking them to take a look! In the absence of a dedicated review dashboard most maintainers manage their pending reviews through email, and those will sometimes get lost in their inbox.

modular-magician · 2022-09-16T17:35:17Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 1 file changed, 1 insertion(+), 2 deletions(-))
Terraform Beta: Diff ( 2 files changed, 15 insertions(+), 5 deletions(-))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2022-09-16T18:23:53Z

Tests analytics

Total tests: 2173
Passed tests 1932
Skipped tests: 238
Failed tests: 3

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccStorageTransferJob_transferOptions|TestAccComputeInstance_soleTenantNodeAffinities|TestAccFirebaserulesRelease_BasicRelease

modular-magician · 2022-09-16T18:29:16Z

Tests passed during RECORDING mode:
TestAccStorageTransferJob_transferOptions[Debug log]
TestAccFirebaserulesRelease_BasicRelease[Debug log]

Tests failed during RECORDING mode:
TestAccComputeInstance_soleTenantNodeAffinities[Error message] [Debug log]

Please fix these to complete your PR
View the build log or the debug log for each test

modular-magician · 2022-09-19T07:01:14Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 1 file changed, 1 insertion(+), 2 deletions(-))
Terraform Beta: Diff ( 3 files changed, 21 insertions(+), 5 deletions(-))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2022-09-19T08:08:23Z

Tests analytics

Total tests: 2174
Passed tests 1934
Skipped tests: 238
Failed tests: 2

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccFirebaserulesRelease_BasicRelease|TestAccComputeInstance_soleTenantNodeAffinities

modular-magician · 2022-09-19T08:10:20Z

Tests passed during RECORDING mode:
TestAccFirebaserulesRelease_BasicRelease[Debug log]

Tests failed during RECORDING mode:
TestAccComputeInstance_soleTenantNodeAffinities[Error message] [Debug log]

Please fix these to complete your PR
View the build log or the debug log for each test

AarshDhokai · 2022-09-27T16:29:45Z

Hey @shuyama1 Could you please review this PR ?

shuyama1 · 2022-09-27T16:54:14Z

Hi @AarshDhokai! Thanks for the ping! Working on reviewing the PR now!

shuyama1 · 2022-09-27T18:16:30Z

Looks like tests are skipped in VCR. Kicked off an independent run: https://ci-oss.hashicorp.engineering/buildConfiguration/GoogleCloudBeta_ProviderGoogleCloudBetaMmUpstream/340405

shuyama1 · 2022-09-27T20:24:07Z

mmv1/third_party/terraform/resources/resource_dataflow_flex_template_job.go.erb

+	//adding wait time for setting all the parameters into state file
+	err = waitForDataflowJobState(d, config, job.Id, userAgent, d.Timeout(schema.TimeoutUpdate), "JOB_STATE_RUNNING")
+	if err != nil {
+		return fmt.Errorf("Error waiting for job with job ID %q to be running: %s", d.Id(), err)


Question: Do we already have d.Id()? Or should it be job.Id in the error message?

Yep actually It should be a job.Id, Thanks for pointing out that.

…time in create function

modular-magician · 2022-09-30T10:32:35Z

Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are.

Diff report:

Terraform GA: Diff ( 1 file changed, 1 insertion(+), 2 deletions(-))
Terraform Beta: Diff ( 3 files changed, 25 insertions(+), 9 deletions(-))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2022-09-30T11:17:38Z

Tests analytics

Total tests: 2193
Passed tests 1948
Skipped tests: 236
Failed tests: 9

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccStorageTransferJob_transferOptions|TestAccDataflowFlexTemplateJob_withServiceAccount|TestAccDataflowFlexTemplateJob_streamUpdateFail|TestAccDataflowFlexTemplateJob_streamUpdate|TestAccDataflowFlexTemplateJob_basic|TestAccComputeInstance_soleTenantNodeAffinities|TestAccFirebaserulesRelease_BasicRelease|TestAccBigtableAppProfile_bigtableAppProfileAnyclusterExample|TestAccBigtableAppProfile_bigtableAppProfileMulticlusterExample

modular-magician · 2022-09-30T11:29:10Z

Tests passed during RECORDING mode:
TestAccStorageTransferJob_transferOptions[Debug log]
TestAccDataflowFlexTemplateJob_withServiceAccount[Debug log]
TestAccDataflowFlexTemplateJob_streamUpdateFail[Debug log]
TestAccDataflowFlexTemplateJob_streamUpdate[Debug log]
TestAccDataflowFlexTemplateJob_basic[Debug log]
TestAccFirebaserulesRelease_BasicRelease[Debug log]
TestAccBigtableAppProfile_bigtableAppProfileAnyclusterExample[Debug log]
TestAccBigtableAppProfile_bigtableAppProfileMulticlusterExample[Debug log]

Tests failed during RECORDING mode:
TestAccComputeInstance_soleTenantNodeAffinities[Error message] [Debug log]

Please fix these to complete your PR
View the build log or the debug log for each test

shuyama1

Thanks for making the changes! LGTM in general!
Let's keep skipping the tests in VCR that are skipped earlier, as I don't think they will be working well in VCR.

modular-magician · 2022-10-03T06:51:27Z

Hi there, I'm the Modular magician - !

Diff report

Terraform GA: Diff ( 1 file changed, 1 insertion(+), 2 deletions(-))
Terraform Beta: Diff ( 3 files changed, 21 insertions(+), 5 deletions(-))
TF Validator: Diff ( 2 files changed, 3 insertions(+), 3 deletions(-))

modular-magician · 2022-10-03T07:34:15Z

Tests analytics

Total tests: 2197
Passed tests 1953
Skipped tests: 240
Failed tests: 4

Action taken

Triggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed tests

TestAccComputeInstance_soleTenantNodeAffinities|TestAccCGCSnippet_eventarcWorkflowsExample|TestAccFirebaserulesRelease_BasicRelease|TestAccSqlDatabaseInstance_mysqlMajorVersionUpgrade

modular-magician · 2022-10-03T08:09:56Z

Tests passed during RECORDING mode:
TestAccFirebaserulesRelease_BasicRelease[Debug log]
TestAccSqlDatabaseInstance_mysqlMajorVersionUpgrade[Debug log]

Tests failed during RECORDING mode:
TestAccComputeInstance_soleTenantNodeAffinities[Error message] [Debug log]
TestAccCGCSnippet_eventarcWorkflowsExample[Error message] [Debug log]

Please fix these to complete your PR
View the build log or the debug log for each test

shuyama1

LGTM! Thank you!
Kicked off the skipped test run on TC: https://ci-oss.hashicorp.engineering/buildConfiguration/GoogleCloudBeta_ProviderGoogleCloudBetaMmUpstream/342310
Will merge the PR after confirming the tests pass.

AarshDhokai added 3 commits September 16, 2022 15:19

added changes of data source compute snapshot

3e0b036

reintroduce a deprecated attribute labels

1c02046

changed docs for flex template job labels by removing deprecation note

5eb033a

modular-magician requested a review from melinath September 16, 2022 17:24

updated the test file

20a9d7a

AarshDhokai marked this pull request as ready for review September 19, 2022 08:30

melinath requested review from a team and shuyama1 and removed request for melinath and a team September 20, 2022 23:33

shuyama1 reviewed Sep 27, 2022

View reviewed changes

remove skipIfVcr func in tests and change job.Id in error of waiting …

d1acd4d

…time in create function

modular-magician requested a review from shuyama1 September 30, 2022 10:21

shuyama1 requested changes Sep 30, 2022

View reviewed changes

added skipIfVcr func in resource_dataflow_flex_template_job_test

b2fa740

modular-magician requested a review from shuyama1 October 3, 2022 05:23

AarshDhokai added 2 commits October 3, 2022 06:25

commit for testing

cc2dca8

fixing typo

fbce497

shuyama1 approved these changes Oct 3, 2022

View reviewed changes

shuyama1 merged commit 44a4b66 into GoogleCloudPlatform:main Oct 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix dataflow flextemplate job labels #6559

Fix dataflow flextemplate job labels #6559

AarshDhokai commented Sep 16, 2022 •

edited by shuyama1

Loading

modular-magician commented Sep 16, 2022

modular-magician commented Sep 16, 2022

modular-magician commented Sep 16, 2022

modular-magician commented Sep 16, 2022

modular-magician commented Sep 16, 2022

modular-magician commented Sep 19, 2022

modular-magician commented Sep 19, 2022

modular-magician commented Sep 19, 2022

AarshDhokai commented Sep 27, 2022

shuyama1 commented Sep 27, 2022

shuyama1 commented Sep 27, 2022

shuyama1 Sep 27, 2022

AarshDhokai Sep 30, 2022

modular-magician commented Sep 30, 2022

modular-magician commented Sep 30, 2022

modular-magician commented Sep 30, 2022

shuyama1 left a comment

modular-magician commented Oct 3, 2022

modular-magician commented Oct 3, 2022

modular-magician commented Oct 3, 2022

shuyama1 left a comment

Fix dataflow flextemplate job labels #6559

Fix dataflow flextemplate job labels #6559

Conversation

AarshDhokai commented Sep 16, 2022 • edited by shuyama1 Loading

modular-magician commented Sep 16, 2022

modular-magician commented Sep 16, 2022

modular-magician commented Sep 16, 2022

Diff report:

modular-magician commented Sep 16, 2022

Tests analytics

Action taken

modular-magician commented Sep 16, 2022

modular-magician commented Sep 19, 2022

Diff report:

modular-magician commented Sep 19, 2022

Tests analytics

Action taken

modular-magician commented Sep 19, 2022

AarshDhokai commented Sep 27, 2022

shuyama1 commented Sep 27, 2022

shuyama1 commented Sep 27, 2022

shuyama1 Sep 27, 2022

Choose a reason for hiding this comment

AarshDhokai Sep 30, 2022

Choose a reason for hiding this comment

modular-magician commented Sep 30, 2022

Diff report:

modular-magician commented Sep 30, 2022

Tests analytics

Action taken

modular-magician commented Sep 30, 2022

shuyama1 left a comment

Choose a reason for hiding this comment

modular-magician commented Oct 3, 2022

Diff report

modular-magician commented Oct 3, 2022

Tests analytics

Action taken

modular-magician commented Oct 3, 2022

shuyama1 left a comment

Choose a reason for hiding this comment

AarshDhokai commented Sep 16, 2022 •

edited by shuyama1

Loading