Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Messenger] Add--all option tomessenger:consume#52411

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:7.1fromunknown repositoryDec 4, 2023
Merged

[Messenger] Add--all option tomessenger:consume#52411

fabpot merged 1 commit intosymfony:7.1fromunknown repositoryDec 4, 2023

Conversation

@ghost
Copy link

@ghostghost commentedNov 1, 2023
edited by OskarStark
Loading

QA
Branch?7.1
Bug fix?no
New feature?yes
Deprecations?no
IssuesCloses#52364
LicenseMIT

When implementing this feature the problem with sync transports came out so theif statement for this was needed. I can see someone reported this 2 months ago in#51556. I think this be can fixed properly in a dedicated PR because it requires to dig into MessengerPass I guess.

maelanleborgne, alexislefebvre, asrorbekh, soorajlegend, and ChezGray123 reacted with thumbs up emoji
Copy link
Contributor

@OskarStarkOskarStark left a comment

Choose a reason for hiding this comment

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

Looks good to me

@OskarStarkOskarStark changed the title[Messenger] Add --all option to messenger:consume[Messenger] Add--all option tomessenger:consumeNov 3, 2023
@OskarStarkOskarStark modified the milestones:6.4,7.1Nov 3, 2023
@OskarStark
Copy link
Contributor

This must target 7.1 due to feature freeze period

@fabpot
Copy link
Member

Thank you @javaDeveloperKid.

@fabpotfabpot merged commitdc330b0 intosymfony:7.1Dec 4, 2023
xabbuh added a commit that referenced this pull requestDec 6, 2023
…ith `--all` option (alexandre-daubois)This PR was merged into the 7.1 branch.Discussion----------[Messenger] Fix timing-out test on `messenger:consume` with `--all` option| Q             | A| ------------- | ---| Branch?       | 7.1| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Issues        | -| License       | MITCI times out since#52411, this aims to fix the test that causes this.Commits-------36307a0 [Messenger] Fix test on `messenger:consume` with `--all` option
@Wirone
Copy link
Contributor

Can you consider this as a bug fix and backport it to 6.4 😂 (jk)? This exactly something we need in our app, where we migrate maaaany workers using multiple transports (because our AMQP instance uses multiple exchanges). It would be much better to use--all instead of specifying all the transports (receivers) in the CLI command.

@alexislefebvre
Copy link
Contributor

alexislefebvre commentedApr 9, 2024
edited
Loading

Can you consider this as a bug fix and backport it to 6.4 😂 (jk)? This exactly something we need in our app, where we migrate maaaany workers using multiple transports (because our AMQP instance uses multiple exchanges). It would be much better to use--all instead of specifying all the transports (receivers) in the CLI command.

What if you use 7.1 in your app?

composer require symfony/messenger:^7.1

Update: I just realized that there is a7.1 branch but it isn't released yet. It will bereleased in May, then upgrading should work and allow you to use the--all option.

In the meantime, using a commit or the Composer flag@dev for this package may work.

@Wirone
Copy link
Contributor

@alexislefebvre Thanks for the feedback, but actually I knew it before☺️. I've added@TODO for migrating to 7.1 when it's available and then we'll use--all. Unfortunately@dev is not suitable for our system, which is too big and too important.

@ghost
Copy link
Author

ghost commentedApr 9, 2024
edited by ghost
Loading

@Wirone Another approach would be to copy past this class from 7.1 branch to your respository and register this in your composer,json. Composer will pick up the first class found in this namespace which will be yours.
When you upgrade to 7.1+ one day, you will just remove this class.

P.S. Remember to check that changes between your 6.4.x locked in your composer.lock and 7.1 branch are only the ones from this PR. If not then decide if it's safe to use it.

GromNaN and alexislefebvre reacted with thumbs up emoji

@Wirone
Copy link
Contributor

FYI: I was able to introduce--all in Symfony 6.4 pretty easily -here's how 😎.

OskarStark reacted with heart emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasAwaiting requested review from nicolas-grekas

@OskarStarkOskarStarkAwaiting requested review from OskarStark

Assignees

No one assigned

Projects

None yet

Milestone

7.1

Development

Successfully merging this pull request may close these issues.

[Messenger] Provide a way to consume all transports without specifying/knowing their names

5 participants

@OskarStark@fabpot@Wirone@alexislefebvre@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp