-
Notifications
You must be signed in to change notification settings - Fork 283
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature: Webhooks for Workflow and Task Statuses #115
Feature: Webhooks for Workflow and Task Statuses #115
Conversation
Co-authored-by: ToastedTaco <lambij10@gmail.com> Co-authored-by: BabyBlue0214 <camdenlanier@gmail.com> Co-authored-by: Elijah Spicer <eli.sushi.spicer@gmail.com>
common/src/main/java/com/netflix/conductor/common/metadata/workflow/WorkflowDef.java
Outdated
Show resolved
Hide resolved
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.
As suggested, the status publisher should maintain its own properties the workflow def should not have those.
grpc/src/main/java/com/netflix/conductor/grpc/AbstractProtoMapper.java
Outdated
Show resolved
Hide resolved
java-sdk/src/main/java/com/netflix/conductor/sdk/workflow/def/ConductorWorkflow.java
Outdated
Show resolved
Hide resolved
@@ -62,6 +62,7 @@ include 'java-sdk' | |||
|
|||
// community modules | |||
include 'workflow-event-listener' | |||
include 'task-status-listener' |
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.
task status listener does not work fully and prob. should be a separate PR later.
thank you @CollinDewey for continuing this. |
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.
thanks
Thank you. This was continued by a group of 4 people, listed at the end of the initial PR message. (#115 (comment)) |
Pull Request type
NOTE: Please remember to run
./gradlew spotlessApply
to fix any format violations.Changes in this PR
Describe the new behavior from this PR, and why it's needed
Issue Netflix/conductor-community#212
We require a notification published from Conductor to a callback URL when certain task statuses are reached.
This continues the changes proposed by @charybr in Netflix/conductor-community#228, which originates from the changes made by @techyragu in CiscoM31/conductor#26.
We have added functionally to allow the user to specify a URL to send task and workflow statuses to respectively. The url can be specified in the configuration file used when the user’s specific conductor instance is being spun up. Below is an example of the properties added to the config:
The endpointTask property is where task notifications will be sent. The endpointWorkflow property is where workflow notifications will be sent. The subscribedTaskStatuses property should be set to SCHEDULED as that is the only state change currently being listened for, but this implementation allows for future enhancements to be added for other task notification types to be listened for and specified in this property to be received at the endpointTask endpoint.
The information that will be sent in a task notification is shown in the following example:
Expand
Whenever a workflow is completed, a notification will be sent out to the specified webhook. The information that will be sent in a workflow notification is shown in the following example:
Expand
This functionality is needed to efficiently process requests when workflows are being managed through a 3rd party service. This functionality is also useful to ascertain which tasks commonly fail, and where a long workflow is in execution at any given time.
Alternatives considered
Describe alternative implementation you have considered
An alternative to pushing webhook notifications would be client applications pulling a workflow or task status from Conductor. This approach does not scale if hundreds or thousands of workflows need to be monitored, calling upon each status of which would become a very expensive operation.
Authors @BabyBlue0214 @EJ-S @ToastedTaco @CollinDewey