Skip to content
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 scheduleOptions to BigQuery Data Transfer. #2507

Closed
wants to merge 2 commits into from
Closed

Add scheduleOptions to BigQuery Data Transfer. #2507

wants to merge 2 commits into from

Conversation

pocket7878
Copy link

Adding scheduleOptions to BigQuery Data transfer.
It does not documented yet on TransferConfig document.

But already supported in golang library & API Explorer.

Release Note Template for Downstream PRs (will be copied)

Add `scheduleOptions` to BigQuery Data Transfer.

@modular-magician
Copy link
Collaborator

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. They will authorize it to run through our CI pipeline, which will generate downstream PRs.

Thanks for your contribution! A human will be with you soon.

@tysen, please review this PR or find an appropriate assignee.

@pocket7878 pocket7878 marked this pull request as ready for review October 23, 2019 17:07
Copy link
Contributor

@danawillow danawillow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @pocket7878, sorry for taking so long to get this reviewed! If you're still interested in it, I left a small comment, otherwise the api.yaml changes look good. Would you mind also adding this field into a test? If any of the existing tests make sense to augment with this block you could do that, otherwise it seems reasonable to add a new one. The test file is at https://github.com/GoogleCloudPlatform/magic-modules/blob/master/third_party/terraform/tests/resource_bigquery_data_transfer_config_test.go. Thanks!

required: false
description: |
Options customizing the data transfer schedule.
properties:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've recently started trying to prevent people from being able to write empty blocks, and since this nested object has three optional fields, this would allow that. We can mark the fields as exactly_one_of or at_least_one_of. Do either of those apply here?

Copy link
Contributor

@samouss samouss Oct 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be interested to have this available in Terraform. I understand your point about the empty block. I think at_least_one_of could apply here. If you write the block the intent is to provide one of the options. The three of them have default values so it shouldn't be problematic i.e. startTime defaults to now(), endDate defaults to never and disableAutoScheduling defaults to false.

I'm more than ok to perform the change but I don't think I'm able to push on the PR. Any suggestions on how I can help to make this happen? I can close the branch and create a new PR if required. Let me know!

https://cloud.google.com/bigquery-transfer/docs/reference/datatransfer/rest/v1/projects.locations.transferConfigs#TransferConfig.ScheduleOptions

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR looks pretty abandoned, so I'm going to go ahead and close it out. Looks like someone else picked up the feature at #3895.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants