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

Finish missing sources from Datastream Stream #7115

Closed

Conversation

iperetz-goo
Copy link
Contributor

@iperetz-goo iperetz-goo commented Jan 12, 2023

Add missing fields and behavior from recently implemented Datastream Stream resource PR link

Changes:

  1. Add Postgres & Oracle source configs
  2. Call an additional update after create stream if the desired_state was RUNNING.
  3. Add support for validation results in operation errors

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)


@modular-magician
Copy link
Collaborator

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
Copy link
Collaborator

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

I've detected that you're a community contributor. @c2thorn, 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
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.

Terraform GA: Diff ( 2 files changed, 2072 insertions(+), 337 deletions(-))
Terraform Beta: Diff ( 2 files changed, 2072 insertions(+), 337 deletions(-))
TF Validator: Diff ( 3 files changed, 645 insertions(+), 3 deletions(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 2406
Passed tests 2154
Skipped tests: 251
Failed tests: 1

Action taken

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

@modular-magician
Copy link
Collaborator

Tests passed during RECORDING mode:
TestAccContainerCluster_withInvalidGatewayApiConfigChannel[Debug log]

All tests passed
View the build log or the debug log for each test

@modular-magician
Copy link
Collaborator

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

Terraform GA: Diff ( 2 files changed, 4988 insertions(+), 1386 deletions(-))
Terraform Beta: Diff ( 2 files changed, 4988 insertions(+), 1386 deletions(-))
TF Validator: Diff ( 3 files changed, 1541 insertions(+), 191 deletions(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 2406
Passed tests 2153
Skipped tests: 251
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|TestAccContainerCluster_withInvalidGatewayApiConfigChannel

@modular-magician
Copy link
Collaborator

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

All tests passed
View the build log or the debug log for each test

@modular-magician
Copy link
Collaborator

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
Copy link
Collaborator

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.

1 similar comment
@modular-magician
Copy link
Collaborator

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

Terraform GA: Diff ( 2 files changed, 4988 insertions(+), 1386 deletions(-))
Terraform Beta: Diff ( 2 files changed, 4988 insertions(+), 1386 deletions(-))
TF Validator: Diff ( 3 files changed, 1541 insertions(+), 191 deletions(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 2419
Passed tests 2165
Skipped tests: 252
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|TestAccComputeForwardingRule_update

@modular-magician
Copy link
Collaborator

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

All tests passed
View the build log or the debug log for each test

@modular-magician
Copy link
Collaborator

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

Terraform GA: Diff ( 2 files changed, 4988 insertions(+), 1386 deletions(-))
Terraform Beta: Diff ( 2 files changed, 4988 insertions(+), 1386 deletions(-))
TF Validator: Diff ( 3 files changed, 1541 insertions(+), 191 deletions(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 2419
Passed tests 2166
Skipped tests: 252
Failed tests: 1

Action taken

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

@modular-magician
Copy link
Collaborator

Tests passed during RECORDING mode:
TestAccFirebaserulesRelease_BasicRelease[Debug log]

All tests passed
View the build log or the debug log for each test

@modular-magician
Copy link
Collaborator

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

Terraform GA: Diff ( 3 files changed, 5042 insertions(+), 1395 deletions(-))
Terraform Beta: Diff ( 3 files changed, 5042 insertions(+), 1395 deletions(-))
TF Validator: Diff ( 3 files changed, 1541 insertions(+), 191 deletions(-))

@modular-magician
Copy link
Collaborator

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

Terraform GA: Diff ( 4 files changed, 5474 insertions(+), 1395 deletions(-))
Terraform Beta: Diff ( 4 files changed, 5474 insertions(+), 1395 deletions(-))
TF Validator: Diff ( 3 files changed, 1541 insertions(+), 191 deletions(-))
TF OiCS: Diff ( 8 files changed, 373 insertions(+))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 2425
Passed tests 2168
Skipped tests: 254
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
TestAccComposerEnvironment_UpdateComposerV2|TestAccComposerEnvironment_UpdateComposerV2WithTriggerer|TestAccComposerEnvironment_ComposerV2

@modular-magician
Copy link
Collaborator

Tests failed during RECORDING mode:
TestAccComposerEnvironment_UpdateComposerV2[Error message] [Debug log]
TestAccComposerEnvironment_UpdateComposerV2WithTriggerer[Error message] [Debug log]
TestAccComposerEnvironment_ComposerV2[Error message] [Debug log]

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

Copy link
Member

@c2thorn c2thorn left a comment

Choose a reason for hiding this comment

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

Apologies for the delay. I have a couple of questions. I have not run tests related to this PR yet, since I think they can be run in the automatic VCR tests instead of being skipped. Let me know

Comment on lines 182 to 183
# Random provider
skip_vcr: true
Copy link
Member

Choose a reason for hiding this comment

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

I'm not seeing the random provider used in either of these examples. Should we remove these lines?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -14,4 +14,11 @@
-%>
if err := waitForDatastreamStreamReady(d, config, d.Timeout(schema.TimeoutCreate) - time.Minute); err != nil {
return fmt.Errorf("Error waiting for Stream %q to be NOT_STARTED or RUNNING during creation: %q", d.Get("name").(string), err)
}

if d.Get("state") != d.Get("desired_state") {
Copy link
Member

Choose a reason for hiding this comment

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

This seems to be completely unrelated to adding the other fields. Is the purpose of doing another Update call just a retry of sorts? Have you tested that this is effective?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a bug fix, datastream requires an additional update call when desired_state=RUNNING

main "$@"
Copy link
Member

Choose a reason for hiding this comment

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

Even though it is trivial, can we remove this change to not affect the file history?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@modular-magician
Copy link
Collaborator

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
Copy link
Collaborator

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.

3 similar comments
@modular-magician
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Collaborator

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.

@roaks3
Copy link
Contributor

roaks3 commented Jan 26, 2023

FYI @c2thorn this was accidentally closed, and the new PR is #7175

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants