-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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 eventarc trigger missing field event_data_content_type #8235
Add eventarc trigger missing field event_data_content_type #8235
Conversation
Hello! I am a robot who works on Magic Modules PRs. I've detected that you're a community contributor. @slevenick, a repository maintainer, has been assigned to assist you and help review your changes. ❓ First time contributing? Click here for more detailsYour assigned reviewer will help review your code by:
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. |
service = google_cloud_run_service.default.name | ||
region = "europe-west1" | ||
} | ||
}s |
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.
Typo here may cause test failures
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.
fixed
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 ( 13 files changed, 210 insertions(+), 66 deletions(-)) Missing test reportYour PR includes resource fields which are not covered by any test. Resource: Please add acceptance tests which include these fields. |
Can you try pushing a new commit? This got caught in the GitHub downtime |
/gcbrun |
Tests analyticsTotal tests: Action takenFound 13 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccFilestoreSnapshot_filestoreSnapshotFullExample|TestAccFilestoreSnapshot_filestoreSnapshotBasicExample|TestAccFilestoreInstance_update|TestAccFilestoreInstance_filestoreInstanceFullExample|TestAccFilestoreInstance_filestoreInstanceBasicExample|TestAccFilestoreBackup_update|TestAccFilestoreBackup_filestoreBackupBasicExample|TestAccComputeFirewallPolicyRule_multipleRules|TestAccClouddeployDeliveryPipeline_VerifyDeliveryPipeline|TestAccClouddeployDeliveryPipeline_CanaryDeliveryPipeline|TestAccClouddeployDeliveryPipeline_DeliveryPipeline|TestAccClouddeployDeliveryPipeline_CanaryrunDeliveryPipeline|TestAccClouddeployDeliveryPipeline_CanaryServiceNetworkingDeliveryPipeline |
/gcbrun |
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR hasn't generated any diffs, but I'll let you know if a future commit does. |
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 testsTestAccComputeTargetHttpsProxy_targetHttpsProxyHttpKeepAliveTimeoutExample|TestAccContainerAwsNodePool_BetaBasicHandWritten|TestAccComputeTargetHttpProxy_targetHttpProxyHttpKeepAliveTimeoutExample|TestAccComputeFirewallPolicyRule_multipleRules |
Rerun these tests in REPLAYING mode to catch issues
|
@slevenick @RileyHYZ Hey! Nice job with this one, but is there any update? Would be nice to have it. |
The DCL was upgraded to 1.44.0 but this field doesn't seem to be present. I'm not sure why, as it exists in the YAML file for that resource in this release: https://github.com/GoogleCloudPlatform/declarative-resource-client-library/blame/main/services/google/eventarc/trigger.yaml |
Ah I see what happened. It's present in GA but not beta, so things are a little weird. I'll ping someone to get this resolved |
The change is intended for GA but not beta and the issue should have been resolved with dcl version>1.42.0. I'll resolve the conflict and commit again. |
Change has been included for both GA and Beta |
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 ( 7 files changed, 102 insertions(+), 13 deletions(-)) Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_clouddeploy_delivery_pipeline" "primary" {
serial_pipeline {
stages {
strategy {
canary {
canary_deployment {
postdeploy {
actions = # value needed
}
predeploy {
actions = # value needed
}
}
custom_canary_deployment {
phase_configs {
postdeploy {
actions = # value needed
}
predeploy {
actions = # value needed
}
}
}
runtime_config {
kubernetes {
gateway_service_mesh {
route_update_wait_time = # value needed
}
}
}
}
standard {
postdeploy {
actions = # value needed
}
predeploy {
actions = # value needed
}
}
}
}
}
}
Resource: resource "google_dataplex_asset" "primary" {
resource_spec {
read_access_mode = # value needed
}
}
Resource: resource "google_eventarc_trigger" "primary" {
event_data_content_type = # value needed
}
|
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 ( 7 files changed, 102 insertions(+), 13 deletions(-)) Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_clouddeploy_delivery_pipeline" "primary" {
serial_pipeline {
stages {
strategy {
canary {
canary_deployment {
postdeploy {
actions = # value needed
}
predeploy {
actions = # value needed
}
}
custom_canary_deployment {
phase_configs {
postdeploy {
actions = # value needed
}
predeploy {
actions = # value needed
}
}
}
runtime_config {
kubernetes {
gateway_service_mesh {
route_update_wait_time = # value needed
}
}
}
}
standard {
postdeploy {
actions = # value needed
}
predeploy {
actions = # value needed
}
}
}
}
}
}
Resource: resource "google_dataplex_asset" "primary" {
resource_spec {
read_access_mode = # value needed
}
}
Resource: resource "google_eventarc_trigger" "primary" {
event_data_content_type = # value needed
}
|
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 testsTestAccDataplexAsset_BasicAssetHandWritten|TestAccDataplexAssetIamBindingGenerated|TestAccDataplexAssetIamMemberGenerated|TestAccDataplexAssetIamPolicyGenerated |
|
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.
Looks like updating the DCL has introduced an issue in dataplex asset. Several tests are failing due to large diffs. I'll need to reach out to some people to see what is going on there
Update to resolve the issue on DCL's end is in review. Apologies for the complications! |
The DCL issue should be resolved now with the new release. |
@RileyHYZ you can upgrade the DCL to the new version and the tests should pass now |
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 ( 7 files changed, 102 insertions(+), 13 deletions(-)) Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_clouddeploy_delivery_pipeline" "primary" {
serial_pipeline {
stages {
strategy {
canary {
canary_deployment {
postdeploy {
actions = # value needed
}
predeploy {
actions = # value needed
}
}
custom_canary_deployment {
phase_configs {
postdeploy {
actions = # value needed
}
predeploy {
actions = # value needed
}
}
}
runtime_config {
kubernetes {
gateway_service_mesh {
route_update_wait_time = # value needed
}
}
}
}
standard {
postdeploy {
actions = # value needed
}
predeploy {
actions = # value needed
}
}
}
}
}
}
Resource: resource "google_dataplex_asset" "primary" {
resource_spec {
read_access_mode = # value needed
}
}
Resource: resource "google_eventarc_trigger" "primary" {
event_data_content_type = # value needed
}
|
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 testsTestAccDataplexAsset_BasicAssetHandWritten|TestAccDataplexAssetIamBindingGenerated|TestAccDataplexAssetIamPolicyGenerated|TestAccDataplexAssetIamMemberGenerated |
|
@@ -4,7 +4,7 @@ go 1.19 | |||
|
|||
require ( | |||
cloud.google.com/go/bigtable v1.19.0 | |||
github.com/GoogleCloudPlatform/declarative-resource-client-library v1.44.0 | |||
github.com/GoogleCloudPlatform/declarative-resource-client-library v1.47.0 |
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.
You'll need to update this to 1.48.0
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.
Sorry I didn't see the upgraded version earlier. Just updated it in the new commit. Thanks!
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 ( 7 files changed, 102 insertions(+), 14 deletions(-)) Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_clouddeploy_delivery_pipeline" "primary" {
serial_pipeline {
stages {
strategy {
canary {
canary_deployment {
postdeploy {
actions = # value needed
}
predeploy {
actions = # value needed
}
}
custom_canary_deployment {
phase_configs {
postdeploy {
actions = # value needed
}
predeploy {
actions = # value needed
}
}
}
runtime_config {
kubernetes {
gateway_service_mesh {
route_update_wait_time = # value needed
}
}
}
}
standard {
postdeploy {
actions = # value needed
}
predeploy {
actions = # value needed
}
}
}
}
}
}
Resource: resource "google_dataplex_asset" "primary" {
resource_spec {
read_access_mode = # value needed
}
}
Resource: resource "google_eventarc_trigger" "primary" {
event_data_content_type = # value needed
}
|
Tests analyticsTotal tests:
|
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.
Thanks!
…udPlatform#8235) * upgrade dcl version to v1.43.0 * add overrides samples for trigger event_data_content_type field * Fix typo * Upgrade dcl version to v1.47.0 * Upgrade dcl version to v1.48.0 to fix issues in previous version. --------- Co-authored-by: Riley <rileyzhou@google.com>
…udPlatform#8235) * upgrade dcl version to v1.43.0 * add overrides samples for trigger event_data_content_type field * Fix typo * Upgrade dcl version to v1.47.0 * Upgrade dcl version to v1.48.0 to fix issues in previous version. --------- Co-authored-by: Riley <rileyzhou@google.com>
This PR upgrades dcl version to v.1.48.0 and adds a field event_data_content_type in eventarc trigger.
fixes hashicorp/terraform-provider-google#14597
If this PR is for Terraform, I acknowledge that I have:
make test
andmake lint
in the generated providers to ensure it passes unit and linter tests.Release Note Template for Downstream PRs (will be copied)