-
Notifications
You must be signed in to change notification settings - Fork 59
Feature: Add command to detect new queues #188
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
base: main
Are you sure you want to change the base?
Conversation
rhoerr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good to me. Can you confirm that this detects amqp discrepancies too? Or can it be expanded for that purpose?
I had a case just last week where a RabbitMQ container rebuilt at the host, and lost all the queues from memory. Had to run setup to fix it. (I don't think Magento self-heals in that situation--may be wrong.) It would help if the command could detect that.
|
@rhoerr It doesnt cover that yet. My original commit had a secondary command that checks against amqp directly but was affected by cache giving false results and I ran out of time on my machine so stripped it out for now rather than commiting junk. But I may extend it later in the night. (Command worked in that if functioned, but new queues only got seen on a cache clean cause I tried to use a different process to the original command) Can check here for an idea of what it looked like: 8fcc1cc before being stipped (The justification was that infra failure should result in an external process anyways, but would be nice to have - I have faced that issue in the past myself hence its original inclusion) |
|
Actually, let me change this up a little. I'm going to put the command in MessageQueue, and have MysqlMQ and Amqp modules hydrate a changeDetectors like attribute via DI so I dont have to worry about creating weird dependencies. |
Moved command from MysqlMq to MessageQueue and allowed for DI ChangeDetector files so MysqlMq and Amqp can add their own individual functions without creating dependency issues Added amqp change detector that does not get affected by cache like original implementation. This ensures your RabbitMQ/AmazonMQ/etc can be checked for inconsistences (Say due to config loss/migration)
|
@rhoerr There we go, updated it while kids where sitting down for dinner. Refactored + added amqp checking so if your RMQ gets nuked it should inform you the queues are out of date on the command run |
rhoerr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to test yet, but visually this code looks good to me. Thank you Francis.
Description
The
setup:db:statuscommand does not pick up newly defined or missing queues, this means anyone trying to get close to true green/blue deployments has no quick way to check if a setup:upgrade is required if a new queue (mysql or amqp) has been added and no other changes are made.This PR adds the console command
queue:config:status, which checks for new queues. In line with other status commands; 0 = no new queues, exit code 2 = new unconfigured queues.Important architecture info: The command resides in MessageQueue, and it uses DI to hydrate any ChangeDetectors - which there are 2 of, one in MysqlMq and one on Amqp, this avoids any additional module dependency. MysqlMq checks if the queue exists in the DB; Amqp, if you have rabbitmq/amazonmq/etc, checks if the queue exists in the broker.
Related Pull Requests
Follows on from the similar PR #186 as a concentrated effort to improve Blue/Green deployment efforts.
Manual testing scenarios
bin/magento queue:config:status## exit code 2bin/magento setup:upgradebin/magento queue:config:status## exit code 0git checkout -- app/code/Magento/ImportExport/etc/queue_topology.xmlQuestions or comments
Contribution checklist (*)
Prexisting unit & integration test failures stop me from checking all tests passing box - though the unit tests related to new code do all pass.