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

[Console][Messenger] addRunCommandMessage andRunCommandMessageHandler#49814

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:6.4fromkbond:messenger-command
Jul 30, 2023

Conversation

@kbond
Copy link
Member

@kbondkbond commentedMar 25, 2023•
edited
Loading

QA
Branch?6.3
Bug fix?no
New feature?yes
Deprecations?no
Ticketsn/a
LicenseMIT
Doc PRtodo

Similar to#49813, when using the scheduler it could be useful to execute commands.

Usage

useSymfony\Component\Console\Exception\RunCommandFailedException;useSymfony\Component\Console\Messenger\RunCommandMessage;try {$context =$bus->dispatch(newRunCommandMessage('my:command'));$context->output;// string - output of command$context->exitCode;// int - the exit codecatch(RunCommandFailedException$e) {// if exit code is non-zero or command threw exception$e->context->output;// string - output of command$e->context->exitCode;// int - the exit code$e->getPrevious();// null|\Throwable exception command threw if applicable}// "never" fail$context =$bus->dispatch(newRunCommandMessage('my:command', throwOnNonSuccess:false, catchExceptions:true));

TODO:

  • wire up
  • tests

OskarStark, valtzu, and chapterjason reacted with thumbs up emoji
@kbondkbond requested a review fromchalasr as acode ownerMarch 25, 2023 17:58
@carsonbotcarsonbot added Status: Needs Review Console Feature Messenger RFCRFC = Request For Comments (proposals about features that you want to be discussed) labelsMar 25, 2023
@carsonbotcarsonbot added this to the6.3 milestoneMar 25, 2023
@carsonbotcarsonbot changed the title[Console][Messenger][RFC] addExecuteCommand andExecuteCommandHandler[Console][Messenger] addExecuteCommand andExecuteCommandHandlerMar 25, 2023
@kbond
Copy link
MemberAuthor

Like#49813 (comment), maybeCommandMessage andCommandMessageHandler would be better names?

Korbeil reacted with thumbs up emoji

@kbondkbond removed the RFCRFC = Request For Comments (proposals about features that you want to be discussed) labelMar 27, 2023
$input = new StringInput($message->input);
$output = new BufferedOutput();

$this->application->setCatchExceptions($message->catchExceptions);
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Should I clone the application here instead of having the service not-shared?

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe try/finally to modify state on the shared service?

kbond reacted with thumbs up emoji
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

I don't think I can do this as there's no way to access the current state ofApplication::$catchExceptions.

@kbondkbondforce-pushed themessenger-command branch 2 times, most recently from848bdb7 tof712569CompareMarch 27, 2023 15:56
@kbond
Copy link
MemberAuthor

kbond commentedMar 28, 2023•
edited
Loading

One thing I think is missing: making the output available if there's an exception. Consider a system that tracks messages and emails an admin on failure. I can attest that this is valuable information.

What about wrapping the caught exception in a newCommandFailedException that contains the output?

In#49813 this is possible because the thrown exception (ProcessFailedException) includes theProcess instance which makes the output available.

) {
}

public function __toString(): string
Copy link
Contributor

Choose a reason for hiding this comment

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

$input = new StringInput($message->input);
$output = new BufferedOutput();

$this->application->setCatchExceptions($message->catchExceptions);
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe try/finally to modify state on the shared service?

kbond reacted with thumbs up emoji
@nicolas-grekasnicolas-grekas modified the milestones:6.3,6.4May 23, 2023
@kbond
Copy link
MemberAuthor

kbond commentedJul 28, 2023•
edited
Loading

I've updated this PR:

  1. Renamed the message toRunCommandMessage.
  2. AddedRunCommandContext to be returned from the handler
  3. AddedRunCommandFailedException that's thrown on failure that wraps the original exception and containsRunCommandContext
  4. Configuration for throwingRunCommandFailedException on non-successful exit codes

(updated PR description to show usage)

@kbondkbond changed the title[Console][Messenger] addExecuteCommand andExecuteCommandHandler[Console][Messenger] addRunCommandMessage andRunCommandMessageHandlerJul 28, 2023
Copy link
Member

@fabpotfabpot left a comment

Choose a reason for hiding this comment

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

LGTM,@kbond I've made a rename suggestion.

@fabpot
Copy link
Member

Thank you@kbond.

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

Reviewers

@fabpotfabpotfabpot approved these changes

@chalasrchalasrAwaiting requested review from chalasrchalasr is a code owner

+1 more reviewer

@ro0NLro0NLro0NL left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

6.4

Development

Successfully merging this pull request may close these issues.

5 participants

@kbond@fabpot@ro0NL@nicolas-grekas@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp