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

Add "Messenger auto_setup" checker #300

Merged

Conversation

M-arcus
Copy link
Contributor

@M-arcus M-arcus commented Dec 6, 2024

No description provided.

@schneider-felix
Copy link
Member

@frosh-automation fix-cs

@schneider-felix
Copy link
Member

Will this report a false positive for rabbitMQ?

@M-arcus
Copy link
Contributor Author

M-arcus commented Dec 6, 2024

@schneider-felix
Copy link
Member

Alright, got it. Can we add the same for rabbitMQ/Redis?

@M-arcus
Copy link
Contributor Author

M-arcus commented Dec 8, 2024

RabbitMQ and Redis messenger are not included by default, and so the classes for the transport are not loaded. How should this be implemented?

@shyim
Copy link
Member

shyim commented Dec 8, 2024

I would just check if the query string contains that auto_setup parameter and not look so much into detail

@schneider-felix
Copy link
Member

I would just check if the query string contains that auto_setup parameter and not look so much into detail

I agree. This is probably enough for 99% of setups.

@M-arcus
Copy link
Contributor Author

M-arcus commented Dec 9, 2024

Changed it to a check that looks for auto_setup bool in the query part of the dsn

Copy link
Member

@schneider-felix schneider-felix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could simplify this a bit by just extracting the query part. But other than that. LGTM

…SetupChecker.php

Co-authored-by: Felix Schneider <69912882+schneider-felix@users.noreply.github.com>
@M-arcus
Copy link
Contributor Author

M-arcus commented Dec 9, 2024

@frosh-automation fix-cs

@schneider-felix schneider-felix merged commit bb7e1b4 into FriendsOfShopware:main Dec 9, 2024
3 checks passed
@schneider-felix
Copy link
Member

@M-arcus thank you very much for your contribution.

@M-arcus M-arcus deleted the messenger-auto-setup branch December 9, 2024 09:09
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

Successfully merging this pull request may close these issues.

3 participants