Fix issue #12883#8738
Conversation
Make one CloudResourceManager call per monitored_project terraform resource rather than per (monitored_project * "sibling" projects)
|
Oops! It looks like you're using an unknown release-note type in your changelog entries:
Please only use the types listed in https://github.com/GoogleCloudPlatform/magic-modules/blob/master/.ci/RELEASE_NOTES_GUIDE.md. |
|
Hello! I am a robot. It looks like you are a: Community Contributor @hao-nan-li, 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. |
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 1 file changed, 27 insertions(+), 86 deletions(-)) |
hao-nan-li
left a comment
There was a problem hiding this comment.
Looks like there are some build errors.
https://googlecloudplatform.github.io/magic-modules/develop/run-tests/ might help.
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 1 file changed, 28 insertions(+), 86 deletions(-)) |
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 1 file changed, 28 insertions(+), 86 deletions(-)) |
Tests analyticsTotal tests: Action takenFound 8 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccBigtableAppProfile_bigtableAppProfileAnyclusterExample|TestAccBigtableAppProfile_bigtableAppProfileMulticlusterExample|TestAccMonitoringMonitoredProject_monitoringMonitoredProjectBasicExample|TestAccMonitoringMonitoredProject_projectNumShortForm|TestAccMonitoringMonitoredProject_projectNumLongForm|TestAccMonitoringMonitoredProject_monitoringMonitoredProjectLongFormExample|TestAccDataSourceGoogleServiceAccountIdToken_impersonation|TestAccFolderIamPolicy_basic |
|
Rerun these tests in REPLAYING mode to catch issues
|
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 1 file changed, 31 insertions(+), 85 deletions(-)) |
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 2 files changed, 36 insertions(+), 85 deletions(-)) |
Tests analyticsTotal tests: Action takenFound 6 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccMonitoringMonitoredProject_monitoringMonitoredProjectBasicExample|TestAccMonitoringMonitoredProject_projectNumShortForm|TestAccMonitoringMonitoredProject_projectNumLongForm|TestAccMonitoringMonitoredProject_monitoringMonitoredProjectLongFormExample|TestAccDataSourceGoogleServiceAccountAccessToken_basic|TestAccDataSourceGoogleServiceAccountJwt |
|
Rerun these tests in REPLAYING mode to catch issues
Tests failed due to non-determinism or randomness when the VCR replayed the response after the HTTP request was made. Please fix these to complete your PR. If you believe these test failures to be incorrect or unrelated to your change, or if you have any questions, please raise the concern with your reviewer.
|
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 1 file changed, 35 insertions(+), 87 deletions(-)) |
Tests analyticsTotal tests: Action takenFound 6 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccDataprocClusterIamPolicy|TestAccDataprocJobIamPolicy|TestAccMonitoringMonitoredProject_monitoringMonitoredProjectBasicExample|TestAccMonitoringMonitoredProject_projectNumLongForm|TestAccMonitoringMonitoredProject_projectNumShortForm|TestAccMonitoringMonitoredProject_monitoringMonitoredProjectLongFormExample |
|
Rerun these tests in REPLAYING mode to catch issues
|
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 1 file changed, 39 insertions(+), 87 deletions(-)) |
Tests analyticsTotal tests: Action takenFound 5 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccDataprocClusterIamPolicy|TestAccMonitoringMonitoredProject_monitoringMonitoredProjectBasicExample|TestAccMonitoringMonitoredProject_projectNumLongForm|TestAccMonitoringMonitoredProject_projectNumShortForm|TestAccMonitoringMonitoredProject_monitoringMonitoredProjectLongFormExample |
|
Rerun these tests in REPLAYING mode to catch issues
|
| @@ -24,9 +24,6 @@ references: !ruby/object:Api::Resource::ReferenceLinks | |||
| guides: | |||
| 'Official Documentation': 'https://cloud.google.com/monitoring/settings/manage-api' | |||
| api: 'https://cloud.google.com/monitoring/api/ref_v3/rest/v1/locations.global.metricsScopes.projects' | |||
There was a problem hiding this comment.
Line 18 in this file. Delete url to v1/{{name}}
There was a problem hiding this comment.
Thanks! Is there an explanation for why this change is now necessary?
For posterity, here's the (private) conversation that I think led to this recommendation:
I think the problem comes down to whatever is decoding the deletion. In the latest CI run of
TestAccMonitoringMonitoredProject_monitoringMonitoredProjectBasicExample, the deletion request is
DELETE /v1/locations/global/metricsScopes/ci-test-project-188019/projects/locations/global/metricsScopes/1067888929963/projects/102753631080
and we can see the path has the format
v1/locations/global/metricsScopes/<project>/projects/<metric_scope_path>
when we just want
DELETE /v1/<metric_scope_path>which would translate toDELETE /v1/locations/global/metricsScopes/1067888929963/projects/10275363108. I added debug lines to the decoder, and the decoder seems to decode the link just fine.
This leads me to suspect some spooky complexity from deleting the
nested_queryfunctionality, in which case I think I definitely need more support in debugging
There was a problem hiding this comment.
It's mainly due to to deleting the nested_query. I don't know why it was implemented with nested_query before. Let the CI runs complete and see if the proposed solution solved the problem for all failing tests.
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 2 files changed, 41 insertions(+), 89 deletions(-)) |
Tests analyticsTotal tests: Action takenFound 4 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccMonitoringMonitoredProject_monitoringMonitoredProjectBasicExample|TestAccMonitoringMonitoredProject_projectNumLongForm|TestAccMonitoringMonitoredProject_monitoringMonitoredProjectLongFormExample|TestAccMonitoringMonitoredProject_projectNumShortForm |
|
Rerun these tests in REPLAYING mode to catch issues
|
* Fix issue GoogleCloudPlatform#12883 Make one CloudResourceManager call per monitored_project terraform resource rather than per (monitored_project * "sibling" projects) * Add comment * Correct build errors * Fix build errors * Add debug logs * Fix printf * Fix match between TF resource and API response * Fix from non-matching to matching condition * Correct documentation * Correct documentation * Fix go build issues * Convert projectNumber to string * Use correct string conversion * Move comment * Add debug logs to monitored project encoder * correct delete_url
* Fix issue GoogleCloudPlatform#12883 Make one CloudResourceManager call per monitored_project terraform resource rather than per (monitored_project * "sibling" projects) * Add comment * Correct build errors * Fix build errors * Add debug logs * Fix printf * Fix match between TF resource and API response * Fix from non-matching to matching condition * Correct documentation * Correct documentation * Fix go build issues * Convert projectNumber to string * Use correct string conversion * Move comment * Add debug logs to monitored project encoder * correct delete_url
Fixed hashicorp/terraform-provider-google#12883
Make one CloudResourceManager call per monitored_project terraform resource rather than per (monitored_project * "sibling" projects)
If this PR is for Terraform, I acknowledge that I have:
make testandmake lintin the generated providers to ensure it passes unit and linter tests.Release Note Template for Downstream PRs (will be copied)