8000 [HttpClient][Messenger] add `PingWebhookMessage` and `PingWebhookMessageHandler` by kbond · Pull Request #49815 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[HttpClient][Messenger] add PingWebhookMessage and PingWebhookMessageHandler #49815

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 30, 2023
Merged

[HttpClient][Messenger] add PingWebhookMessage and PingWebhookMessageHandler #49815

merged 1 commit into from
Jul 30, 2023

Conversation

@kbond
Copy link
Member
@kbond kbond commented Mar 25, 2023 •
edited
Loading
Q A
Branch? 6.4
Bug fix? no
New feature? yes
Deprecations? no
Tickets n/a
License MIT
Doc PR todo

With symfony/scheduler, it could be useful to ping some kind of uptime monitoring service like ohdearapp.

Usage

use Symfony\Component\HttpClient\Messenger\PingWebhookMessage;

$bus->dispatch(new PingWebhookMessage('GET', 'https://example.com')); // simple ping, throws HttpExceptionInterface on 3xx/4xx/5xx

$bus->dispatch(new PingWebhookMessage('GET', 'https://example.com', throw: false)); // ping, but does not throw HttpExceptionInterface on 3xx/4xx/5xx

$bus->dispatch(new PingWebhookMessage('GET', 'https://example.com', [
    'headers' => ['X-FOO => 'bar'], // any HttpClientInterface options
]));

TODO:

  • wire up
  • tests

@carsonbot carsonbot added Status: Needs Review Feature HttpClient Messenger RFC RFC = Request For Comments (proposals about features that you want to be discussed) labels Mar 25, 2023
@carsonbot carsonbot added this to the 6.3 milestone Mar 25, 2023
@carsonbot carsonbot changed the title [HttpClient][Messenger][RFC] add PingWebhook and PingWebhookHandler [HttpClient][Messenger] add PingWebhook and PingWebhookHandler Mar 25, 2023
@kbond
Copy link
Member Author
kbond commented Mar 25, 2023

Perhaps this would make more sense in the Webhook component? I wasn't sure if this would be in the same scope.

@kbond kbond force-pushed the messenger-ping-webhook branch from 3d34fdd to 12e3f2e Compare March 27, 2023 15:56
@kbond kbond removed the RFC RFC = Request For Comments (proposals about features that you want to be discussed) label Mar 27, 2023
@kbond kbond mentioned this pull request Mar 27, 2023
Merged
2 tasks
welcoMattic
welcoMattic approved these changes Mar 30, 2023
View reviewed changes
Copy link
Member
@welcoMattic welcoMattic left a comment

Choose a reason for hiding this comment

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

I think HttpClient is the good place for this 👍

@fabpot
Copy link
Member
fabpot commented Apr 10, 2023

The main idea here is to make the ping retry-able, isn't it?

@kbond
Copy link
Member Author
kbond commented Apr 10, 2023

The main idea here is to make the ping retry-able, isn't it?

You mean the logic for catching/throwing exceptions? The main idea was to know if a ping failed.

@ro0NL
Copy link
Contributor
ro0NL commented Apr 10, 2023

all messages are already retry-able https://symfony.com/doc/current/messenger.html#retries-failures

@kbond
Copy link
Member Author
kbond commented Apr 10, 2023

all messages are already retry-able https://symfony.com/doc/current/messenger.html#retries-failures

The scheduler transport does not support retries.

@nicolas-grekas nicolas-grekas modified the milestones: 6.3, 6.4 May 23, 2023
nicolas-grekas
nicolas-grekas reviewed Jun 9, 2023
View reviewed changes
nicolas-grekas
nicolas-grekas reviewed Jun 9, 2023
View reviewed changes
nicolas-grekas
nicolas-grekas approved these changes Jun 9, 2023
View reviewed changes
Copy link
Member
@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

LGTM after minor changes. Thanks for the PR.

@nicolas-grekas
Copy link
Member
nicolas-grekas commented Jun 9, 2023

Please improve the PR description a bit so that it's easier to start the doc. 🙏

@nicolas-grekas
Copy link
Member
nicolas-grekas commented Jun 28, 2023

(rebase needed + comments pending)

@kbond
Copy link
Member Author
kbond commented Jun 28, 2023

Yep, this is on my list!

@kbond kbond force-pushed the messenger-ping-webhook branch from 12e3f2e to c6a4885 Compare July 28, 2023 12:00
@kbond
Copy link
Member Author
kbond commented Jul 28, 2023 •
edited
Loading

Comments addressed, I think this is ready.

kbond
kbond commented Jul 28, 2023
View reviewed changes
@kbond kbond changed the title [HttpClient][Messenger] add PingWebhook and PingWebhookHandler [HttpClient][Messenger] add PingWebhookMessage and PingWebhookMessageHandler Jul 28, 2023
@kbond kbond force-pushed the messenger-ping-webhook branch from 8629503 to 3fc4175 Compare July 28, 2023 13:46
nicolas-grekas
nicolas-grekas approved these changes Jul 28, 2023
View reviewed changes
@kbond kbond mentioned this pull request Jul 29, 2023
Closed
@fabpot fabpot force-pushed the messenger-ping-webhook branch from 43a9c4c to f0644d9 Compare July 30, 2023 09:34
@fabpot
Copy link
Member
fabpot commented Jul 30, 2023

Thank you @kbond.

@fabpot fabpot merged commit bfb5963 into symfony:6.4 Jul 30, 2023
@fabpot fabpot mentioned this pull request Jul 30, 2023
Closed
This was referenced Oct 21, 2023
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fabpot fabpot fabpot approved these changes

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

@welcoMattic welcoMattic welcoMattic approved these changes

+1 more reviewer

@ro0NL ro0NL ro0NL left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Feature HttpClient Messenger Status: Reviewed

Projects

None yet

Milestone

6.4

Development

Successfully merging this pull request may close these issues.

6 participants

@kbond @fabpot @ro0NL @nicolas-grekas @welcoMattic @carsonbot
0