Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include cli parameters in ceQueue definition #16

Open
mlambley opened this issue Feb 20, 2020 · 2 comments
Open

Include cli parameters in ceQueue definition #16

mlambley opened this issue Feb 20, 2020 · 2 comments

Comments

@mlambley
Copy link

Could the ceQueue definition include, optionally, the fields interval, limit, requeue, runonce?

Something like:

<ceQueue name="product.updates" broker="mysql"
    messageSchema="int"
    consumerInterface="Rcason\MqExample\Model\ExampleConsumer" 
    interval="250"
    limit="1200"
    requeue="1"
    runonce="0"
/>

And then in StartConsumerCommand if those options are not specified on the command line, it looks for them in the xml, and if not found in either the command line or the xml then it uses the default values?

Thanks.

@renatocason
Copy link
Owner

Hi @mlambley ,

First of all thanks for using the extension and for your contribution. I took a quick look and the changes look fine, I would be a bit tempted of making the new parameters optional in the interface, to make the changes not backward incompatible. It's not considered best practice, but it would make the update seamless for the extension users. What's your idea on that?

Regards,
Renato

@mlambley
Copy link
Author

mlambley commented Feb 24, 2020

I don't think that will be enough. Sure I can say int $maxRetries = 5, int $retryInterval = 0 in BrokerInterface, and do the same for the other interfaces, but the breaking changes are more extensive than that.

Anyone who has implemented their own Broker, Config or QueueMessageRepository from the interfaces will break because their function definitions will be different. Also anyone who passes in the $maxRetries parameter to the MessageRepository constructor will find their system not working as expected.

If those are acceptable risks let me know and I'll proceed with the optional parameter changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants