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

fix(pubsub): allow updating topic schema fields individually #7362

Merged

Conversation

hongalex
Copy link
Member

@hongalex hongalex commented Feb 3, 2023

Currently, the entirety of SchemaSettings must be updated when calling UpdateTopic. Instead, allow updating these fields individually (e.g. first/last revision) without needing to specify schema ID or encoding.

@hongalex hongalex requested review from a team and shollyman as code owners February 3, 2023 23:01
@product-auto-label product-auto-label bot added size: s Pull request size is small. api: pubsub Issues related to the Pub/Sub API. labels Feb 3, 2023
@hongalex hongalex added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Feb 4, 2023
@hongalex hongalex removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Feb 8, 2023
if t.proto.SchemaSettings == nil {
t.proto.SchemaSettings = &pb.SchemaSettings{}
}
t.proto.SchemaSettings.LastRevisionId = req.Topic.SchemaSettings.LastRevisionId
default:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is also possible to just have schema_settings, which will update all of them. Though I guess the Go library is setting each field explicitly?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah each field is set explicitly so updating all of them at once is possible too.

@@ -407,7 +407,18 @@ func (t *Topic) updateRequest(cfg TopicConfigToUpdate) *pb.UpdateTopicRequest {
}
if cfg.SchemaSettings != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does one remove the schema config entirely?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I missed this. I added a section that handles clearing SchemaSettings when it detects the zero value, similar to MessageStoragePolicy.

@product-auto-label product-auto-label bot added size: m Pull request size is medium. and removed size: s Pull request size is small. labels Feb 24, 2023
Copy link
Member Author

@hongalex hongalex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed comments. Also, kokoro is failing because of a unrelated sample issue, not because of any tests related to pubsub

@@ -407,7 +407,18 @@ func (t *Topic) updateRequest(cfg TopicConfigToUpdate) *pb.UpdateTopicRequest {
}
if cfg.SchemaSettings != nil {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I missed this. I added a section that handles clearing SchemaSettings when it detects the zero value, similar to MessageStoragePolicy.

if t.proto.SchemaSettings == nil {
t.proto.SchemaSettings = &pb.SchemaSettings{}
}
t.proto.SchemaSettings.LastRevisionId = req.Topic.SchemaSettings.LastRevisionId
default:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah each field is set explicitly so updating all of them at once is possible too.

@hongalex hongalex added the automerge Merge the pull request once unit tests and other checks pass. label Feb 27, 2023
@gcf-merge-on-green gcf-merge-on-green bot merged commit f09e059 into googleapis:main Feb 27, 2023
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Feb 27, 2023
gcf-merge-on-green bot pushed a commit that referenced this pull request Mar 13, 2023
🤖 I have created a release *beep* *boop*
---


## [1.29.0](https://togithub.com/googleapis/google-cloud-go/compare/pubsub/v1.28.0...pubsub/v1.29.0) (2023-03-13)


### Features

* **pubsub:** Add google.api.method.signature to update methods ([aeb6fec](https://togithub.com/googleapis/google-cloud-go/commit/aeb6fecc7fd3f088ff461a0c068ceb9a7ae7b2a3))
* **pubsub:** Add REST client ([06a54a1](https://togithub.com/googleapis/google-cloud-go/commit/06a54a16a5866cce966547c51e203b9e09a25bc0))
* **pubsub:** Add schema evolution methods and fields ([ee41485](https://togithub.com/googleapis/google-cloud-go/commit/ee41485860bcbbd09ce4e28ee6ddca81a5f17211))
* **pubsub:** Add support for schema revisions ([#7295](https://togithub.com/googleapis/google-cloud-go/issues/7295)) ([369b16f](https://togithub.com/googleapis/google-cloud-go/commit/369b16f9525f9ac9a0811c66ce61eda9f6c566e4))
* **pubsub:** Add temporary_failed_ack_ids to ModifyAckDeadlineConfirmation ([aeb6fec](https://togithub.com/googleapis/google-cloud-go/commit/aeb6fecc7fd3f088ff461a0c068ceb9a7ae7b2a3))
* **pubsub:** Make INTERNAL a retryable error for Pull ([aeb6fec](https://togithub.com/googleapis/google-cloud-go/commit/aeb6fecc7fd3f088ff461a0c068ceb9a7ae7b2a3))


### Bug Fixes

* **pubsub/pstest:** Fix panic on undelivered message ([#7377](https://togithub.com/googleapis/google-cloud-go/issues/7377)) ([98dd29d](https://togithub.com/googleapis/google-cloud-go/commit/98dd29d372073605145f78f08205a9786c698881))
* **pubsub:** Allow updating topic schema fields individually ([#7362](https://togithub.com/googleapis/google-cloud-go/issues/7362)) ([f09e059](https://togithub.com/googleapis/google-cloud-go/commit/f09e059e3203de5294648d7434d5e65626a6dff5))
* **pubsub:** Dont compare revision fields in schema config test ([#7317](https://togithub.com/googleapis/google-cloud-go/issues/7317)) ([e364f7a](https://togithub.com/googleapis/google-cloud-go/commit/e364f7abfe3ec8fc20db78abcdaeaaf27d19269c))
* **pubsub:** Fix bug with AckWithResult with exactly once disabled ([#7319](https://togithub.com/googleapis/google-cloud-go/issues/7319)) ([c88fbdf](https://togithub.com/googleapis/google-cloud-go/commit/c88fbdf299205e8118b347430cf66540ffa68b27))
* **pubsub:** Pipe revision ID in name in DeleteSchemaRevision ([#7519](https://togithub.com/googleapis/google-cloud-go/issues/7519)) ([e211635](https://togithub.com/googleapis/google-cloud-go/commit/e211635216e553a9a6b00f9e8f2c5d2082ff68a8))


### Documentation

* **pubsub:** Add x-ref for ordering messages docs: Clarify subscription expiration policy ([aeb6fec](https://togithub.com/googleapis/google-cloud-go/commit/aeb6fecc7fd3f088ff461a0c068ceb9a7ae7b2a3))
* **pubsub:** Clarify BigQueryConfig PERMISSION_DENIED state ([aeb6fec](https://togithub.com/googleapis/google-cloud-go/commit/aeb6fecc7fd3f088ff461a0c068ceb9a7ae7b2a3))
* **pubsub:** Clarify subscription description ([aeb6fec](https://togithub.com/googleapis/google-cloud-go/commit/aeb6fecc7fd3f088ff461a0c068ceb9a7ae7b2a3))
* **pubsub:** Mark revision_id in CommitSchemaRevisionRequest deprecated ([2fef56f](https://togithub.com/googleapis/google-cloud-go/commit/2fef56f75a63dc4ff6e0eea56c7b26d4831c8e27))
* **pubsub:** Replacing HTML code with Markdown docs: Fix PullResponse description docs: Fix Pull description ([aeb6fec](https://togithub.com/googleapis/google-cloud-go/commit/aeb6fecc7fd3f088ff461a0c068ceb9a7ae7b2a3))
* **pubsub:** Update Pub/Sub topic retention limit from 7 days to 31 days ([aeb6fec](https://togithub.com/googleapis/google-cloud-go/commit/aeb6fecc7fd3f088ff461a0c068ceb9a7ae7b2a3))

---
This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
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. size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants