[Messenger] Use newer version of Beanstalkd bridge library #60040
Conversation
| "pda/pheanstalk": "^4.0", | ||
| "pda/pheanstalk": "^5.1|^7.0", |
There was a problem hiding this comment.
These are the versions that have the disconnect() method, which is needed for the reconnect mechanism. Also, v7 supports only PHP 8.3+, while v5 supports PHP 8.1+.
src/Symfony/Component/Messenger/Bridge/Beanstalkd/Transport/Connection.php
Outdated
Show resolved
Hide resolved
| return $this->client->watchOnly($this->tube)->reserveWithTimeout($this->timeout); | ||
| if ($this->client->watch($this->tube) > 1) { | ||
| foreach ($this->client->listTubesWatched() as $tube) { | ||
| if ((string) $tube !== (string) $this->tube) { | ||
| $this->client->ignore($tube); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The watchOnly method was removed, this implements similar behavior.
| $job = $this->client->useTube($this->tube)->put( | ||
| $this->client->useTube($this->tube); |
There was a problem hiding this comment.
Some methods like useTube are no longer chainable.
| @@ -503,3 +517,7 @@ public function testSendWithRoundedDelay() | |||
| $connection->send($body, $headers, $delay); | |||
| } | |||
| } | |||
|
|
|||
| interface PheanstalkInterface extends PheanstalkPublisherInterface, PheanstalkSubscriberInterface, PheanstalkManagerInterface | |||
There was a problem hiding this comment.
The PheanstalkInterface was split into multiple interfaces. Since the client implements all of them, it's hard to mock. This is a helper interface to make mocking easier.
| try { | ||
| return $command(); | ||
| } catch (ConnectionException) { | ||
| $this->client->disconnect(); |
There was a problem hiding this comment.
Not sure what to do about the Psalm errors. Even though the disconnect() method wasn't added to the interfaces in v5 (to avoid BC breaks), it was added to the Pheanstalk class itself, which is what's used here, so the method always exists. I could use method_exists(), but that feels redundant. The method was added to the interfaces in v7, but that version requires PHP 8.3.
|
Thank you @HypeMC. |
…alk (HypeMC) This PR was merged into the 7.3 branch. Discussion ---------- [Messenger] Fix integration with newer versions of Pheanstalk | Q | A | ------------- | --- | Branch? | 7.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | - | License | MIT Follow-up to #60040. When I updated the Pheanstalk library, I missed the fact that version 4 internally tracked the [used](https://github.com/pheanstalk/pheanstalk/blob/1459f2f62dddfe28902e0584708417dddd79bd70/src/Pheanstalk.php#L316-L324)/[watched](https://github.com/pheanstalk/pheanstalk/blob/1459f2f62dddfe28902e0584708417dddd79bd70/src/Pheanstalk.php#L329-L337) tube. This was removed in newer versions, so now every call to `useTube()` or `watch()` sends a command to Beanstalkd, leading to unnecessary bandwidth consumption. This PR adds simple tracking logic to avoid redundant calls to those functions. Commits ------- b766607 [Messenger] Fix integration with newer version of Pheanstalk
Currently, the Beanstalkd bridge supports version 4 of
pda/pheanstalk, which is no longer maintained. The latest version is v7, and the actively maintained versions appear to be v5 and v7. Version 7 was released just a week after v6, and from what I can tell, v6 is not supported as no patch versions have ever been released for it.This PR bumps support to the maintained versions. I've split it into two commits to make reviewing easier:
Connectionclass.I'm not sure which version of Symfony this should target. It's not exactly a new feature, but the changes are significant.