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

pubsub: cannot clear bigquery subscription #8037

Closed
hongalex opened this issue Jun 1, 2023 · 1 comment · Fixed by #8040
Closed

pubsub: cannot clear bigquery subscription #8037

hongalex opened this issue Jun 1, 2023 · 1 comment · Fixed by #8040
Assignees
Labels
api: pubsub Issues related to the Pub/Sub API. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@hongalex
Copy link
Member

hongalex commented Jun 1, 2023

In the same vein as #7979, there is a flaw with the logic of clearing BigQuery subscriptions. Setting a zero valued BigQuery subscription is a signal to clear the bigquery subscription, but right now Update passes on the empty BigQueryConfig instead of a nil one along with the update mask.

The flaw comes from

if cfg.BigQueryConfig != nil {
psub.BigqueryConfig = cfg.BigQueryConfig.toProto()

If BigQueryConfig is not nil (in this case, the sentinel clearing value), we immediately try to convert into the proto struct as is, rather than transforming it as nil. The fix here is to update the toProto method to convert a zero valued config to nil. To simplify, we only really need to check if config.Table is equal to the empty string, since we don't allow partial updates.

I'm not sure if PushConfig suffers the same problem or not, but will investigate this separately.

@hongalex hongalex added the triage me I really want to be triaged. label Jun 1, 2023
@product-auto-label product-auto-label bot added the api: pubsub Issues related to the Pub/Sub API. label Jun 1, 2023
@hongalex hongalex self-assigned this Jun 1, 2023
@hongalex hongalex added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. api: pubsub Issues related to the Pub/Sub API. and removed api: pubsub Issues related to the Pub/Sub API. triage me I really want to be triaged. labels Jun 1, 2023
@hongalex
Copy link
Member Author

hongalex commented Jun 6, 2023

Re: PushConfig, we don't have the same problem for some reason. Setting an empty PushConfig is enough to clear it though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: pubsub Issues related to the Pub/Sub API. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant