prolic / HumusAmqp

PHP 7.4 AMQP library
https://humusamqp.readthedocs.io/
MIT License
76 stars 17 forks source link

removes recursive auto setup flag from exchange and queue factory #57

Closed oqq closed 6 years ago

oqq commented 7 years ago

This PR removes the $autoSetupFabric from inner calls in ExchangeFactory and QueueFactory. Since some users could depending on this auto setup, this could be a bc break.

My proposal is it to remove that flag from ExchangeFactory at all and use instead only the auto_setup_fabric from exchange config.

Another approach could be to add a "do not auto setup binded exchanges" config parameter. But yes, this would be strange.

It is not always possible to know the whole config for exchanges. So this call would throw an exception if the provided exchange config does not match exchanges already exists on the rabbit broker.

So it should be possible to only bind a queue to an existing exchange with only know his name (not his whole configuration). Also a dead letter binding to an existing exchange should work.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 92.686% when pulling 38cf6ceab7f55286e7983b309c918f8c250786bc on oqq:bc_break/remove_auto_setup into e1f298debdfdf609c742258ccfee37c149dde57c on prolic:master.

prolic commented 7 years ago

This behaviour is indeed wanted. If you are running with "auto_setup_fabric" - everything is automatically set up for you. I would suggest to turn it on only for development.

oqq commented 7 years ago

So, if this is the case, it would not be possible to use humus factories to setup queues with binding on exchanges not defined in same application at all. Even in production with the SetupFabricCommand.

Given the following architecture: An RabbitMQ Broker is running on another machine with a fixed setup for a RPC architecture with exchanges, his fallbacks and some queues. Now, some RPC-Server implementations would provide a service by creating a queue with binding on the "All requests exchange" for a particular routing key. This Servers are defined from different companies, without whole knowledge about the configuration for "All requests exchange".

All I know is the naming of exchanges which should be used for rpc queue bindings. If I would provide such a service with a php application using the HumusAmqp Package, I would done it with this configuration:

return [
    'humus' => [
        'amqp' => [
            'exchange' => [
                'rpc' => [
                    'name' => 'rpc',
                    'connection' => 'amqp.connection.default',
                ],
                'rpc.result' => [
                    'name' => 'rpc.result',
                    'connection' => 'amqp.connection.default',
                ],
            ],
            'queue' => [
                'rpc.dev' => [
                    'name' => 'rpc.dev',
                    'connection' => 'amqp.connection.default',
                    'durable' => false,
                    'auto_delete' => true,
                    'exchanges' => [
                        'rpc' => [
                            [
                                'routing_keys' => [
                                    'dev',
                                ],
                            ],
                        ],
                    ],
                ],
                'rpc.result' => [
                    'connection' => 'amqp.connection.default',
                    'durable' => false,
                    'exclusive' => true,
                    'exchanges' => [
                        'rpc.result' => [],
                    ],
                ],
            ],
        ],
    ],
];

Now, if I run the SetupFabricCommand for this queues, I would get something like:

[PhpAmqpLib\Exception\AMQPProtocolChannelException]
  PRECONDITION_FAILED - inequivalent arg 'alternate-exchange' for exchange 'rpc' in vhost '/': received none but current is the value 'rpc.fallback' of type 'longstr'

Also, this queues have to be gone if the rpc is not running. If I could not set "auto_setup_fabric" or using the SetupFabricCommand, I have to build and run an own setup factory right before the beginning of every rpc server start.

So, if I get you right, all provided factories of this package are only meant to be used in an dev environment but not for production? Or is my setup not correct?

oqq commented 7 years ago

Oh and it is also not possible to create an exclusive queue without 'auto_setup_fabric' => true.

prolic commented 6 years ago

Thanks @oqq, you brought up some valid points. I need a few days to think about a good solution and try to improve this at the weekend.

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 92.686% when pulling 38cf6ceab7f55286e7983b309c918f8c250786bc on oqq:bc_break/remove_auto_setup into e1f298debdfdf609c742258ccfee37c149dde57c on prolic:master.