Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Messenger] Add "non sendable" stamps#31471
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
4a027a1 to42e4158Comparesrc/Symfony/Component/Messenger/Transport/Serialization/RemoveNonSendableStampsTrait.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Messenger/Transport/Serialization/RemoveNonSendableStampsTrait.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
sroze left a comment
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.
The use-case is definitely there 👍
src/Symfony/Component/Messenger/Transport/Serialization/RemoveNonSendableStampsTrait.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
42e4158 to37a43d6CompareFixes a bug where Symfony serialization of the AmqpReceivedStamp sometimes caused problems.
37a43d6 to34e7781Compareweaverryan commentedMay 13, 2019
Moved implementation to a new I've only added the new "non-sendable stamp interface" to 3 transport-specific "received" stamps. Generally speaking, Ithink that stampsshould be sent, unless there is a clear reason otherwise. Allowing stamps to be sendable (the default)can make the messages bigger when being retried after failure. But they also allow for a really beautiful "history" as you can look at an Envelope and see exactly what happened to it over time. Anyways, this is ready to go! |
weaverryan commentedMay 20, 2019
This is still ready to go. |
fabpot commentedMay 21, 2019
Thank you@weaverryan. |
This PR was merged into the 4.3 branch.Discussion----------[Messenger] Add "non sendable" stamps| Q | A| ------------- | ---| Branch? | 4.3| Bug fix? | yes| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#31460| License | MIT| Doc PR | not neededFixes a bug where Symfony serialization of the AmqpReceivedStamp sometimes caused problems.It's still a mystery why the `AmqpReceivedStamp` caused a segfault *sometimes* when going through the Symfony serializer or the `VarDumper`. But, that stamp really didn't need to be sent on redelivery anyways.I don't love making the removal the responsibility of the serializers, but it didn't work well anywhere else.Cheers!Commits-------34e7781 Adding a new NonSendableStampInterface to avoid sending certain stamps
Fixes a bug where Symfony serialization of the AmqpReceivedStamp sometimes caused problems.
It's still a mystery why the
AmqpReceivedStampcaused a segfaultsometimes when going through the Symfony serializer or theVarDumper. But, that stamp really didn't need to be sent on redelivery anyways.I don't love making the removal the responsibility of the serializers, but it didn't work well anywhere else.
Cheers!