Skip to content

CAMEL-13401: adding a webhook meta-component and first implementation with Telegram#2867

Merged
nicolaferraro merged 5 commits intoapache:masterfrom
nicolaferraro:webhook-3
Apr 15, 2019
Merged

CAMEL-13401: adding a webhook meta-component and first implementation with Telegram#2867
nicolaferraro merged 5 commits intoapache:masterfrom
nicolaferraro:webhook-3

Conversation

@nicolaferraro
Copy link
Member

As per subject. This adds the capability for components to self register to receive push notifications instead of doing polling.

Registration is automatic by default, but it will be disabled in camel k where there will be a specific process to register webhooks.

In knative, components supporting webhooks can scale to 0 ;)

There are few things I'm not sure about... for example, the MultiRestConsumer class.

public class Routes extends RouteBuilder {
@Override
public void configure() {
restConfiguration().contextPath("/camel");
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the rest configuration needed, as /camel is default

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, /camel is the default when using servlet in spring-boot, but the rest configuration is not aware of that. The webhook component needs to know it because it has to reconstruct the external URL of the service and I though that the contextPath of the restConfiguration was the right place.

@nicolaferraro
Copy link
Member Author

Thanks @davsclaus for the review, I'll apply the changes.

@nicolaferraro
Copy link
Member Author

Should be better now.

@oscerd
Copy link
Contributor

oscerd commented Apr 15, 2019

@nicolaferraro you can go ahead and merge.

@nicolaferraro nicolaferraro merged commit 83f58a3 into apache:master Apr 15, 2019
@nicolaferraro
Copy link
Member Author

Thanks!

@@ -0,0 +1,58 @@
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
Copy link
Member

Choose a reason for hiding this comment

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

Need to add the license header here.

protected void doStop() throws Exception {
super.doStop();

if (configuration.isWebhookAutoRegister()) {
Copy link
Member

Choose a reason for hiding this comment

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

It could be better to unregister the webhook before the WebhookEndpoint stop itself.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, going to fix.

@@ -0,0 +1,83 @@
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
Copy link
Member

Choose a reason for hiding this comment

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

ASF License header.

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.

4 participants