Always attempt to listen for notifications #47209
Conversation
|
I've also added a test but i'm not sure it is worth the effort |
d2dd112 to
5700919
Compare
|
If we're sure that it's safe to call |
|
@goetas Can you have a look at the tests, https://github.com/symfony/symfony/runs/7711529784?check_suite_focus=true might be related to this change. |
| public function reset() | ||
| { | ||
| parent::reset(); | ||
| $this->unlisten(); | ||
| } |
There was a problem hiding this comment.
don't we want to keep this? we still want to unlisten on reset, don't we?
There was a problem hiding this comment.
i have reverted a portion of this. I have also verified that calling UNLISTEN multiple times makes no harm
eaa0b31 to
b462665
Compare
| }; | ||
|
|
||
| // dbal 2.x | ||
| if (interface_exists(Result::class)) { |
There was a problem hiding this comment.
I had to do this weird thing to test against dbal 2.x and dbal 3.x. I'm not proud of it but i do not have a better idea
There was a problem hiding this comment.
You could maybe duplicate the test to remove the if (and skip if not using the good version). But IMHO that's good enough as-is.
I have fixed the test, it was because i had locally dbal 3.x instlled while on CI 2.x is runing. https://github.com/symfony/symfony/pull/47209/files#r939714144 does a different mocking by trying to detect which dbal is running |
b462665 to
72ac5bf
Compare
|
Thank you @goetas. |
…es with custom consumption priorities. Replay symfony#47209 on top of this code change.
as discussed in #45056 (comment), when a middleware closes the connection, the messenger consumer will not receive notifications when a new record is inserted.
Since is safe to call
LISTENmultiple times, we can avoid having to keep tack if we did call or not the listen command, we call it always so we are always sure to get notifications with new messages are added.