Skip to content
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

Flyte webhooks #4431

Draft
wants to merge 44 commits into
base: master
Choose a base branch
from
Draft

Flyte webhooks #4431

wants to merge 44 commits into from

Conversation

pingsutw
Copy link
Member

@pingsutw pingsutw commented Nov 15, 2023

TL;DR

Add a new webhook processor capable of parsing the workflow event and sending the message to an external service, such as Slack.

  1. Flyteadmin sends a task or workflow external event to SNS
  2. SNS push the event to SQS
  3. FlyteAdmin webhook processor pulls the event from SQS
  4. Parse the event, and then render the payload template ({{ name }} succeeded)
  5. Send the event to external service by calling the webhook endpoint

For GCP users, we push the envent to pub/sub service

Note: if you have two webhooks in the admin config, sns should push the message to two different sqs. Each webhook processor will pull the event from a different sqs.

TODO:

  1. Update flytekit notification interface in the Launch plan
lp = LaunchPlan.create(
    "ray_workflow",
    ray_workflow,
    default_inputs={"n": 4},
    notifications=[
        Webhook(
            phases=[WorkflowExecutionPhase.SUCCEEDED],
            payload="{{ workflow.name }} succeeded",
            name="slack",  # Webhook name, slack, team, etc
        )
    ],
)
  1. Read the webhook URL from the secret since some webhook contains sensitive information. e.g. slack
  2. (TBD, for on-prom users) Add a kafka processor that can pull the message from kafka, and then send the message to slack.
  3. (TBD) use webhook in the task level?

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

webhook config

externalEvents:
  enable: true
  type: "aws"
  aws:
    region: "us-west-2"
  eventsPublisher:
    topicName: "arn:aws:sns:us-west-2:590375264460:webhook"
    eventTypes:
      - all
webhooknotifications:
  aws:
    region: "us-west-2"
  webhooks:
    name: slack
    url: https://hooks.slack.com/services/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
    payload: "{{ name }} succeeded"
    processor:
      queueName: webhook
      accountId: "590375264460"

Tracking Issue

#2317

Follow-up issue

NA

pingsutw added 30 commits May 19, 2023 16:14
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
pingsutw and others added 12 commits July 20, 2023 12:46
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Copy link

codecov bot commented Nov 15, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (4f2a164) 59.82% compared to head (b7cb5aa) 58.78%.

❗ Current head b7cb5aa differs from pull request most recent head 70a28b3. Consider uploading reports for the commit 70a28b3 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4431      +/-   ##
==========================================
- Coverage   59.82%   58.78%   -1.04%     
==========================================
  Files         632      232     -400     
  Lines       53644    16340   -37304     
==========================================
- Hits        32091     9606   -22485     
+ Misses      19028     5839   -13189     
+ Partials     2525      895    -1630     
Flag Coverage Δ
unittests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
return s.Config
}

func (s *SlackWebhook) Post(ctx context.Context, payload admin.WebhookMessage) error {
Copy link
Member Author

Choose a reason for hiding this comment

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

Need to rename it to webhookClient or something else? it should work if people give any other kind of webhook endpoints.

@pingsutw pingsutw marked this pull request as draft August 16, 2024 20:17
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.

2 participants