-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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 SQL Backup Run Datasource #4352
Add SQL Backup Run Datasource #4352
Conversation
Read: dataSourceSqlBackupRunRead, | ||
|
||
Schema: map[string]*schema.Schema{ | ||
"backup_id": { |
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.
Requesting sanity check on using backup_id
vs name
for the id
field in https://cloud.google.com/sql/docs/mysql/admin-api/rest/v1beta4/backupRuns#BackupRun
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.
Seems reasonable to me. We have similar things in compute, map_id
and similar as substitutes for id
.
Why would a user want to specify the backup_id
if the point of this resource seems to be to retrieve the id of the most recent backup?
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.
Really just for access to the Computed fields of the resource. Definitely not as useful without adding more of the fields to the datasource, but status
and start_time
may be useful to a user right now.
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=164780" |
Computed: true, | ||
Description: `The status of this run.`, | ||
}, | ||
"most_recent": { |
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.
For context, the AWS equivalent behavior is shown here: https://github.com/hashicorp/terraform-provider-aws/blob/master/aws/data_source_aws_db_snapshot.go#L161
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=164780" |
I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccDataSourceSqlBackupRun_notFound|TestAccDataSourceStorageBucketObjectContent_Basic|TestAccActiveDirectoryDomainTrust_activeDirectoryDomainTrustBasicExample|TestAccContainerCluster_withConfidentialNodes|TestAccContainerCluster_withPrivateClusterConfigMissingCidrBlock|TestAccContainerCluster_withIPv4Error|TestAccContainerNodePool_withKubeletConfig|TestAccDataprocJob_Spark You can view the result here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=164789" |
1 similar comment
I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccDataSourceSqlBackupRun_notFound|TestAccDataSourceStorageBucketObjectContent_Basic|TestAccActiveDirectoryDomainTrust_activeDirectoryDomainTrustBasicExample|TestAccContainerCluster_withConfidentialNodes|TestAccContainerCluster_withPrivateClusterConfigMissingCidrBlock|TestAccContainerCluster_withIPv4Error|TestAccContainerNodePool_withKubeletConfig|TestAccDataprocJob_Spark You can view the result here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=164789" |
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.
I'm not seeing TestAccDataSourceSqlBackupRun_basic
run anywhere... Know why that might be?
Read: dataSourceSqlBackupRunRead, | ||
|
||
Schema: map[string]*schema.Schema{ | ||
"backup_id": { |
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.
Seems reasonable to me. We have similar things in compute, map_id
and similar as substitutes for id
.
Why would a user want to specify the backup_id
if the point of this resource seems to be to retrieve the id of the most recent backup?
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=164863" |
I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccDataSourceGoogleSubnetwork|TestAccDataSourceStorageBucketObjectContent_Basic|TestAccActiveDirectoryDomainTrust_activeDirectoryDomainTrustBasicExample|TestAccComputeFirewall_update|TestAccComputeFirewall_priority|TestAccComputeFirewall_disabled|TestAccComputeFirewall_enableLogging|TestAccContainerCluster_withConfidentialNodes|TestAccContainerCluster_withPrivateClusterConfigMissingCidrBlock|TestAccDataprocJob_Spark You can view the result here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=164866" |
Part of hashicorp/terraform-provider-google#2446
Backup Run datasource is essential for grabbing a most recent backup run ID to restore to.
https://cloud.google.com/sql/docs/mysql/admin-api/rest/v1beta4/backupRuns
most_recent
field behavior inspired by the AWS equivalent https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/db_snapshot#most_recentIf 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)