-
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
Add cmek to spanner database #4699
Conversation
/gcbrun |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 3 files changed, 226 insertions(+), 2 deletions(-)) |
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=182336" |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 4 files changed, 227 insertions(+), 3 deletions(-)) |
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=182349" |
I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccCloudRunDomainMapping_foregroundDeletion|TestAccComputeRouterPeer_advertiseMode You can view the result here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=182441" |
It looks like this is a skip-VCR test; I started a non-VCR test run for the new test here: https://ci-oss.hashicorp.engineering/buildConfiguration/GoogleCloud_ProviderGoogleCloudMmUpstream/183041 |
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.
LGTM assuming the test passes
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.
It looks like the test currently requires google_project_service_identity (beta only) but the field is currently marked as GA.
Ah yeah, that's correct. I just marked the test as beta to avoid this issue in tests. Spanner only has a v1 API so it wouldn't be correct to mark it as beta. I think the right action here is to mark the test as beta but keep the field as GA |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 4 files changed, 227 insertions(+), 1 deletion(-)) |
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=183178" |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 3 files changed, 80 insertions(+), 1 deletion(-)) |
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=183245" |
I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccCloudRunDomainMapping_foregroundDeletion|TestAccComputeBackendService_internalLoadBalancing|TestAccTags You can view the result here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=183249" |
Tests failed during RECORDING mode: TestAccCloudRunDomainMapping_foregroundDeletion Please fix these to complete your PR |
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.
LGTM assuming test passes
Fixes: hashicorp/terraform-provider-google#8822
Also bumps the default request timeout to 2 minutes from 30 seconds. This should fix intermittent request cancelled errors which were showing up fairly frequently on this test
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)