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
feat(pubsub): add topic message retention duration #4520
Conversation
pubsub/subscription.go
Outdated
// messages published to the subscription's topic in the last | ||
// `TopicMessageRetentionDuration` are always available to subscribers. | ||
// | ||
// This field is set only in responses from the server and otherwise ignored. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be clear: this is an "output only" field? I know this isn't a proto field, but just want to clarify with https://google.aip.dev/203#output-only
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this is an output only field, correspondig to https://github.com/googleapis/googleapis/blob/ba30d8097582039ac4cc4e21b4e4baa426423075/google/pubsub/v1/pubsub.proto#L761.
Should I explicitly add this to the comments here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I explicitly add this to the comments here?
It's fine the way it is, but maybe it would be clearer to say This is an output only field, meaning it will only appear in responses from the backend and will be ignored if sent in a request.
You could include the link too if you feel it would be helpful.
No change is really needed, just wanted to clarify for myself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM for general review.
* feat(pubsub): add message retention duration feature * feat(pubsub): add topic message retention duration * comment fixes * switch to optional.Duration * remove staging endpoint * address review comments * remove unused instance of message retention duration in publishsettings * revert sentinel value check * clarify doc comments * fix retention duration comments
This PR adds support for topic retention duration. This is a new field that can be set on topic creation, and can also be changed by calling
topic.Update
. If set to a negative value for anUpdate
, this clears the retention duration from the topic.