-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Added support for adaptive protection to google_compute_security_policy #4846
Added support for adaptive protection to google_compute_security_policy #4846
Conversation
Hello! I am a robot who works on Magic Modules PRs. I have detected that you are a community contributor, so your PR will be assigned to someone with a commit-bit on this repo for initial review. Thanks for your contribution! A human will be with you soon. @c2thorn, please review this PR or find an appropriate assignee. |
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=190742" |
I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccComputeBackendService_withSecurityPolicy|TestAccComputeSecurityPolicy_withAdaptiveProtection|TestAccComputeSecurityPolicy_basic|TestAccComputeSecurityPolicy_withRule|TestAccComputeSecurityPolicy_withRuleExpr|TestAccComputeSecurityPolicy_update You can view the result here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=190743" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @matty-rose,
Recently we've been wanting to restrict the use of beta APIs in the GA provider. Can we change this to use the appropriate client based on the version? We could use a variable to hold config.NewComputeClient(userAgent)
vs config.NewComputeClient(userAgent)
or something to that extent.
@c2thorn that makes a lot of sense, do you have any suggestions/preferences on how to handle all the |
Using https://github.com/GoogleCloudPlatform/magic-modules/blob/master/mmv1/third_party/terraform/resources/resource_dataproc_cluster.go.erb as a reference, I believe we may be able to remove the |
e63754d
to
9eb5017
Compare
9eb5017
to
5ad3fe2
Compare
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=191295" |
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=191295" |
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=191295" |
thanks for that example @c2thorn, have pushed changes and tests are passing locally for both ga and beta providers so hopefully the magician agrees 🤞 |
I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccEventarcTrigger_transport You can view the result here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=191328" |
I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccEventarcTrigger_transport You can view the result here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=191329" |
I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccEventarcTrigger_transport You can view the result here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=191330" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems good from my end, thanks for bearing with us on this tricky change!
Fixes hashicorp/terraform-provider-google#8984
If this PR is for Terraform, I acknowledge that I have:
make test
andmake lint
to ensure it passes unit and linter tests.Release Note Template for Downstream PRs (will be copied)