Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[Messenger] Allow to skip message inFailedMessagesRetryCommand#57270
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
[Messenger] Allow to skip message inFailedMessagesRetryCommand#57270
Uh oh!
There was an error while loading.Please reload this page.
Conversation
carsonbot commentedMay 31, 2024
Hey! I see that this is your first PR. That is great! Welcome! Symfony has acontribution guide which I suggest you to read. In short:
Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change. When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor! I am going to sit back now and wait for the reviews. Cheers! Carsonbot |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
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 dont know deeply the internal of messenger, but if it is accepted as is its a simple solution 👌🏼
FailedMessagesRetryCommandThere 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.
Does this implementation mean that skipping will update the failure date? If yes, is there any way around?
Is there no way to just NACK the message and go to the next one?
Note that the answer might be just "no" by design, I don't remember this aspect :)
Anyone else can tell?
Uh oh!
There was an error while loading.Please reload this page.
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.
@Kazadri Can you rebase on current 7.2 to get rid of the merge commit?
Update CHANGELOG message and help message
d9ce65f to7ec6914Compare
Sure, it's done ! 👍 |
Thank you@Kazadri. |
6d1aa36 intosymfony:7.2Uh oh!
There was an error while loading.Please reload this page.
| $choice = $io->choice('Please select an action', ['retry', 'delete', 'skip'], 'retry'); | ||
| $shouldHandle = $shouldForce || 'retry' === $choice; |
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.
With this change the user is always asked to select an option even if--force was used.
I'm going to prepare a fix
EDIT see#60293
Uh oh!
There was an error while loading.Please reload this page.
When retrying message with the command messenger:failed:retry, we have two options:
But sometimes we have no clue if we can retry it or not but I know that we don't want to delete it yet. In that case we need to stop the command and go id by id. The interactive command is lost. The command should provide a way to skip a message to continue with the rest of the failed message.