migrate bigquerydataset to new API #3125
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
xiaoweim
force-pushed
the
migrate_bigquerydataset_new_API
branch
4 times, most recently
from
b4ebd5e
to
956f9e8
Compare
xiaoweim
changed the title
|
/assign @jingyih |
xiaoweim
force-pushed
the
migrate_bigquerydataset_new_API
branch
from
956f9e8
to
fe0c30a
Compare
pkg/controller/direct/bigquerydataset/bigquerydataset_mappings.go
Outdated
Show resolved
Hide resolved
| out.Location = direct.ValueOf(in.Location) | ||
| out.IsCaseInsensitive = direct.ValueOf(in.IsCaseInsensitive) |
There was a problem hiding this comment.
Did you get a chance to find out the existing usage of this field?
There was a problem hiding this comment.
I can only find out the usage of the resource, but not specific field :(. I just a YAQS question to the bigquery team, hopefully I can get an answer. If this field is too important, we might have to hold off migrating to the recommended API.
pkg/controller/direct/bigquerydataset/bigquerydataset_mappings.go
Outdated
Show resolved
Hide resolved
pkg/controller/direct/bigquerydataset/bigquerydataset_mappings.go
Outdated
Show resolved
Hide resolved
| @@ -253,15 +269,16 @@ func (a *Adapter) Update(ctx context.Context, updateOp *directbase.UpdateOperati | |||
| if desired.Access == nil || len(desired.Access) == 0 { | |||
| resource.Access = a.actual.Access | |||
| } | |||
| updateDatasetCall := a.gcpService.Datasets.Update(parent.ProjectID, datasetId, resource) | |||
| updated, err := updateDatasetCall.Do() | |||
| datasetMetadataToUpdate := BigQueryDatasetMetadata_ToMetadataToUpdate(mapCtx, resource) | |||
There was a problem hiding this comment.
Not sure if it is possible/easier to convert directly from krm.BigQueryDatasetSpec to pb.DatasetMetadataToUpdate in "Update". I feel it might be easier to read and maintain in the future.
There was a problem hiding this comment.
This update does not support updateMask, so I would still need to go through all the non-immutable fields and update the clone of the a.actual object.
I could convert the desired object directly from krm.BigQueryDatasetSpec to pb.DatasetMetadataToUpdate. But I think the problem is that a.actual is of type pb.DatasetMetadata. After I updated the a.actual object with new values from the desired object, I would still need to convert it to pb.DatasetMetadataToUpdate.
xiaoweim
force-pushed
the
migrate_bigquerydataset_new_API
branch
from
fe0c30a
to
f8e8acd
Compare
The New BigQuery API does not support the field
isCaseSensitive.make ready-prto ensure this PR is ready for review.