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.

Sign up for GitHub

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

Jump to bottom

Lease management settings should be consolidated in one place #1914

Open
hongalex opened this issue Apr 26, 2024 · 2 comments
Open

Lease management settings should be consolidated in one place #1914

hongalex opened this issue Apr 26, 2024 · 2 comments
Assignees
@feywind
Labels
api: pubsub Issues related to the googleapis/nodejs-pubsub API. priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@hongalex
Copy link
Member

Currently, controlling min/max ack deadline is done in SubscriberOptions, but configuring the total extension (for a message's ack deadline) is in FlowControlOptions.

These are related settings and should be in the same place.

Separately, flow control options being in a the lease-manager.ts file seems strange, since lease management should only concern itself with modacks rather than flow/concurrency control.
@hongalex hongalex added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. priority: p3 Desirable enhancement or fix. May not be included in next release. labels Apr 26, 2024
@product-auto-label product-auto-label bot added the api: pubsub Issues related to the googleapis/nodejs-pubsub API. label Apr 26, 2024
@feywind
Copy link
Collaborator

feywind commented Apr 29, 2024

As discussed elsewhere, the settings can't be changed until a breaking major. The lease manager changes are planned for an internals cleanup effort.

@feywind
Copy link
Collaborator

feywind commented May 2, 2024

Kinda looks like the lease manager separation is going to help with this issue, too:

#1213

So I might do that sooner rather than later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@feywind feywind

Labels
api: pubsub Issues related to the googleapis/nodejs-pubsub API. priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@hongalex @feywind