Skip to content

[Console][Messenger] Add $seconds to keepalive() methods #58552

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

Merged
merged 1 commit into from
Oct 14, 2024

Conversation

valtzu
Copy link
Contributor

@valtzu valtzu commented Oct 13, 2024

Q A
Branch? 7.2
Bug fix? no
New feature? yes
Deprecations? no
License MIT

Make the transport aware for how long (at minimum) the message should be kept alive.

F.e. when extending SQS visibility timeout, you need to pass the visibility timeout (seconds) as parameter. If you pass a value which is less than alarm interval, SQS would resend the message too early. By making the transport aware of when the next keepalive call will happen we'll be able to do some assertions/clamping to improve DX.

This is a prerequisite for #58483.

@valtzu valtzu requested a review from chalasr as a code owner October 13, 2024 12:33
@carsonbot carsonbot added this to the 7.2 milestone Oct 13, 2024
@carsonbot carsonbot changed the title [Messenger][Console] Add $seconds to keepalive() methods [Console][Messenger] Add $seconds to keepalive() methods Oct 13, 2024
Copy link
Member

@chalasr chalasr left a comment

Choose a reason for hiding this comment

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

The component changelog needs to mention the new method

@valtzu
Copy link
Contributor Author

valtzu commented Oct 13, 2024

The component changelog needs to mention the new method

As this whole feature is not released yet, I thought the changes here would be covered by the initial changelog entries about adding keepalive / alarm support since from component user perspective it's a single new feature – but ok, I will add those. 👍

@OskarStark OskarStark added the ❄️ Feature Freeze Important Pull Requests to finish before the next Symfony "feature freeze" label Oct 14, 2024
@fabpot
Copy link
Member

fabpot commented Oct 14, 2024

Thank you @valtzu.

@fabpot fabpot merged commit 661bda1 into symfony:7.2 Oct 14, 2024
8 of 10 checks passed
@fabpot fabpot mentioned this pull request Oct 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Console Feature Messenger ❄️ Feature Freeze Important Pull Requests to finish before the next Symfony "feature freeze" Status: Reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants