feat: sync BigQueryDataTransferConfig to pinned googleapis version
#3335
Conversation
Re-running the type generator and mapper generator for bigquerydatatransferconfig
jingyih
force-pushed
the
sync_bigquerydatatransferconfig_to_pinned_version
branch
from
ac3f334 to
39dbcbe
Compare
jingyih
changed the title
BigQueryDataTransferConfig to pinned googleapis versionBigQueryDataTransferConfig to pinned googleapis version
| type EventDrivenSchedule struct { | ||
| // Pub/Sub subscription used to receive events. | ||
| // Only Google Cloud Storage data source support this option. | ||
| // Format: projects/{project}/subscriptions/{subscription} |
There was a problem hiding this comment.
nit: comment needs to be updated to resource reference
| /* NOTYET | ||
| // A list of messages that carry the error details. There is a common set of | ||
| // message types for APIs to use. | ||
| Details []Any `json:"details,omitempty"` |
There was a problem hiding this comment.
Can I ask why we comment this out? The Any struct seems to be good (though the name is confusing) https://github.com/GoogleCloudPlatform/k8s-config-connector/pull/3335/files#diff-bed80b84c2e759102b2daf0a568eea2dd1d8309634dea509f245721d4684a350R102
There was a problem hiding this comment.
I didn’t spend much time on this field because it’s an output field that can be added later if needed. This PR was created quickly to unblock another PR and was put together in a very short timeframe.
| } | ||
| return nil, fmt.Errorf("error reading referenced PubSubSubscription %v: %w", nn, err) | ||
| } | ||
| projectID, err := ResolveProjectID(ctx, reader, u) |
There was a problem hiding this comment.
Have you tried this against real GCP?
The PubSubSubscription uses the project-id annotation so this will always end up as not-found.
cc @gemmahou to improve the project reference (focus area)
There was a problem hiding this comment.
Not sure if I understand. Currently the code does look for the project-id annotation.
But I did not try this against real GCP, I can add a test later.
|
|
||
| go run . generate-mapper \ | ||
| --service google.cloud.bigquery.datatransfer.v1 \ | ||
| --api-version bigquerydatatransfer.cnrm.cloud.google.com/v1alpha1 | ||
| --api-version bigquerydatatransfer.cnrm.cloud.google.com/v1beta1 |
| --resource BigQueryDataTransferConfig:TransferConfig | ||
| --api-version bigquerydatatransfer.cnrm.cloud.google.com/v1beta1 \ | ||
| --resource BigQueryDataTransferConfig:TransferConfig \ | ||
| --skip-scaffold-files # skipping because the files were generated using a previous pattern, making them incompatible with the new scaffolding approach. |
There was a problem hiding this comment.
Oh! I didn't know this flag. nice!
There was a problem hiding this comment.
(reviewed the following code) Ah.. no wonders... 😅
There was a problem hiding this comment.
Yeah this is kind of needed for re-running the codegen commands. Because we updated both the naming of the scaffolded files and the location of the external reference and identity code.
There was a problem hiding this comment.
Could you update the direct guide ?
There was a problem hiding this comment.
Ideally, new resources shouldn't require this flag. It's only intended for legacy direct resources created while the code template was being updated. We want the codegen tool to be able to be rerun without requiring the flag. I'm looking into the possibility of adding a presubmit check to ensure the tool can be rerun on PR.
|
/approve Looks good especially the flag in scifi tooling. I think we want to resolve the project-reference, but not a blocker to this PR. |
|
[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 |
|
/hold In case you can take a look at the comment. Feel free to unhold and resolve in this PR or follow up PR |
|
/lgtm |
google-oss-prow
bot
merged commit 05b63d0
into
GoogleCloudPlatform:master
Update the
BigQueryDataTransferConfigresource up to date with the pinned git version.This introduces the following new fields.
Adding new optional fields in v1beta1 should be fine. The conversion can be handled automatically by kube-apiserver.
Issue: #3254