AMPQ Interop migration. #516
Conversation
|
@stloyd what do you think of this? |
|
@stloyd could you please create a 2.x branch? |
|
What can I do to proceed with this? |
|
Is it planned to continue work on the branch? |
|
@Evgen14 I am ready to push this forward, just want to make sure the work is not wasted and would be finally merged. |
|
@makasim yes, it could be a new version or fork |
|
I don’t want work on a fork |
|
@makasim can I somehow help to promote this PR? |
|
Who is in charge of the bundle and making decisions? I am ready to work on this more, just want to be sure it is merged and you maintainers like the idea. |
|
Thank you for the effort. I was checking AMQP interop and it looks promising. I have few comments about DI and tests. I will add them later as I am on mobile now. |
|
@skafandri any news? I'd be happy to work on this (have some time for that now). |
|
@skafandri I tried to keep it as BC as possible. Lots of other things could be improved with breaking changes. Though I think we should start with as minimin as possible breaking changes and do bigger things later. |
| { | ||
| $ch = $this->getChannel(); | ||
|
|
||
| // TODO fix it |
| // TODO fix it | ||
| $context = new \Enqueue\AmqpLib\AmqpContext($this->conn); | ||
| $context->setDelayStrategy(new RabbitMqDlxDelayStrategy()); | ||
| $rp = new \ReflectionProperty($context, 'channel'); |
| $rp = new \ReflectionProperty($context, 'channel'); | ||
| $rp->setAccessible(true); | ||
| $rp->setValue($context, $ch); | ||
| $rp->setAccessible(false); |
| $queue = $context->createQueue($queueName); | ||
| $topic = $context->createTopic($exchangeName); | ||
|
|
||
| // TODO I don't know why we cannot bind to the default exchange. Should be investigated. |
| @@ -2,6 +2,8 @@ | |||
|
|
|||
| namespace OldSound\RabbitMqBundle\RabbitMq; | |||
|
|
|||
| use Enqueue\AmqpLib\AmqpContext; | |||
| use Interop\Queue\PsrProcessor; | |||
There was a problem hiding this comment.
This is not a PSR. The className is confusing
| @@ -27,6 +27,54 @@ public function setDeliveryMode($deliveryMode) | |||
| return $this; | |||
| } | |||
|
|
|||
| /** | |||
| * @return null | |||
There was a problem hiding this comment.
What is the point to use a property if the value is always null?
| @@ -2,6 +2,11 @@ | |||
|
|
|||
| namespace OldSound\RabbitMqBundle\RabbitMq; | |||
|
|
|||
| use Enqueue\AmqpTools\RabbitMqDlxDelayStrategy; | |||
| use Interop\Amqp\AmqpContext; | |||
There was a problem hiding this comment.
Package missing in composer's requirements
|
@jderusse I am trying to keep the migration on to amqp-interop as backward compatible as possible. that's why I added obviously, I am not trying to do a wrapper. I am not going to change the configuration, cli commands, or any public interface in any way. no deps on any enqueue package at the end. One would like to use bunny or event amqp ext because of their better performance. |
The migration to AMQP Interop brings several benefits:
The goal is to do migration as backward compatible as possible.
At the end, there wont be a dependency on enqueue, only amqp-interop