fix: datasetRef is not a required field in BigQueryDataTransferConfig #3777
Conversation
| @@ -291,7 +291,7 @@ func (a *Adapter) Update(ctx context.Context, updateOp *directbase.UpdateOperati | |||
| if !reflect.DeepEqual(desired.DataSourceId, a.actual.DataSourceId) { | |||
| return fmt.Errorf("BigQueryDataTransferConfig %s/%s data source ID cannot be changed", u.GetNamespace(), u.GetName()) | |||
| } | |||
| if !reflect.DeepEqual(desired.Destination, a.actual.Destination) { | |||
| if desired.Destination != nil && !reflect.DeepEqual(desired.Destination, a.actual.Destination) { | |||
There was a problem hiding this comment.
Just curious, is there a generated destination if it is unspecified by the user?
There was a problem hiding this comment.
I don't think so. But let me run the test against real GCP to double check.
update: Just tried. No, the service will not generate the destination.
| @@ -283,6 +129,7 @@ X-Xss-Protection: 0 | |||
| "write_disposition": "WRITE_APPEND" | |||
| }, | |||
| "schedule": "first sunday of quarter 00:00", | |||
| "state": 2, | |||
There was a problem hiding this comment.
Just curious, is this added state the result of not setting destinationDatasetId?
There was a problem hiding this comment.
Good catch. I am actually not sure. The "state" is hardcoded as PENDING (2) in the mock. Not sure why it was missing in the previous http log.
|
/lgtm Do you happen to know why we introduce the "required" flag to the datasetRef field? is that from a GCP change? |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: yuwenma The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
google-oss-prow
bot
merged commit afd21ab
into
GoogleCloudPlatform:master
Fix: #3772