Fix : google_bigquery_data_transfer_config should support EventDrivenSchedule #15094
Fix : google_bigquery_data_transfer_config should support EventDrivenSchedule #15094hassannmoussaa wants to merge 9 commits intoGoogleCloudPlatform:mainfrom
Conversation
|
Hello! I am a robot. Tests will require approval from a repository maintainer to run. Googlers: For automatic test runs see go/terraform-auto-test-runs. @trodge, 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. |
|
I need to work on coverage test before you review please @trodge |
|
@trodge for your review |
|
@trodge This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
|
@trodge are we ready here ? |
|
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.
Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_bigquery_data_transfer_config" "primary" {
schedule_options {
event_driven_schedule {
pubsub_subscription = # value needed
}
}
}
|
Tests analyticsTotal tests: 1 Click here to see the affected service packages
🟢 All tests passed! View the build log |
trodge
left a comment
There was a problem hiding this comment.
We'll need a test that includes the new field.
|
@trodge Acceptance test added |
mmv1/templates/terraform/examples/bigquerydatatransfer_config_pub_sub.tf.tmpl
Show resolved
Hide resolved
|
@trodge This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @trodge This PR has been waiting for review for 1 week. Please take a look! Use the label |
|
Apologies for delated review while I was OOO, running the tests again 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.
Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_bigquery_data_transfer_config" "primary" {
schedule_options {
event_driven_schedule {
pubsub_subscription = # value needed
}
}
}
|
Tests analyticsTotal tests: 1 Click here to see the affected service packages
🟢 All tests passed! View the build log |
There was a problem hiding this comment.
What is the reason for excluding tests for this resource?
Also, I think the example syntax may be incorrect. See above comment for how it should be formatted for consistency with other examples and the resource structure.
|
Sure ill be solving this asap |
|
@trodge making Original Code What do you think |
|
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.
Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_bigquery_data_transfer_config" "primary" {
schedule_options {
event_driven_schedule {
pubsub_subscription = # value needed
}
}
}
|
Tests analyticsTotal tests: 1 Click here to see the affected service packages
🟢 All tests passed! View the build log |
mmv1/templates/terraform/examples/bigquerydatatransfer_config_pub_sub.tf.tmpl
Outdated
Show resolved
Hide resolved
...party/terraform/services/bigquerydatatransfer/resource_bigquery_data_transfer_config_test.go
Outdated
Show resolved
Hide resolved
...party/terraform/services/bigquerydatatransfer/resource_bigquery_data_transfer_config_test.go
Outdated
Show resolved
Hide resolved
|
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.
|
Tests analyticsTotal tests: 2 Click here to see the affected service packages
Action takenFound 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
|
🔴 Tests failed during RECORDING mode: 🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR. |
|
New failure: I reccomend not attempting to set permissions using resources in the test config, as this can interfere with concurrent runs. Instead, add the necessary permissions to |
|
@trodge This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
trodge
left a comment
There was a problem hiding this comment.
See above comment re: no IAM resources in test configs
|
Fixed in last commit |
|
Approved the build just now, it should finish running in an hour or so. |
|
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.
|
Tests analyticsTotal tests: 2 Click here to see the affected service packages
Action takenFound 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
|
🔴 Tests failed during RECORDING mode: 🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR. |
| data "google_project" "project" { | ||
| } | ||
|
|
||
| resource "google_project_iam_member" "permissions" { |
There was a problem hiding this comment.
See previous comment about not using IAM resources directly in test configs.
| resource "google_bigquery_dataset" "my_dataset" { | ||
| depends_on = [google_project_iam_member.permissions] | ||
|
|
||
| dataset_id = "my_dataset-%s" |
There was a problem hiding this comment.
| dataset_id = "my_dataset-%s" | |
| dataset_id = "my_dataset_%s" |
- is not allowed in this resource's id.
|
Will be solving this today |
|
@trodge This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
trodge
left a comment
There was a problem hiding this comment.
See above (requesting changes again to stop bot reminders)
|
@hassannmoussaa, this PR is waiting for action from you. If no action is taken, this PR will be closed in 28 days. Please address any comments or change requests, or re-request review from a core reviewer if no action is required. This notification can be disabled with the |
|
@hassannmoussaa, this PR is waiting for action from you. If no action is taken, this PR will be closed in 14 days. Please address any comments or change requests, or re-request review from a core reviewer if no action is required. This notification can be disabled with the |
|
@hassannmoussaa, this PR is waiting for action from you. If no action is taken, this PR will be closed in 2 weekdays. Please address any comments or change requests, or re-request review from a core reviewer if no action is required. This notification can be disabled with the |
|
@hassannmoussaa, this PR is being closed due to inactivity. |

Description
google_bigquery_data_transfer_config should support EventDrivenSchedule
New Configuration :
References
PR to solve issue: hashicorp/terraform-provider-google#24274