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

feat: add webhook call to other events #612

Merged
merged 4 commits into from
Oct 15, 2024
Merged

feat: add webhook call to other events #612

merged 4 commits into from
Oct 15, 2024

Conversation

bthari
Copy link
Contributor

@bthari bthari commented Oct 7, 2024

Description

Previously on #601 I added webhook for model version endpoint related event, and in here the event will be expanded into a model, model endpoint, model version related event, as we also want to have an action (from other service) to be triggered if these events happen.

Modifications

  • created another package for webhook interface
  • add event for:
    • model created
    • model endpoint created/updated/deleted
    • model version created/updated/deleted
  • change previous event name of on-model-version-* to on-version-endpoint-*

Tests

Checklist

  • Added PR label
  • Added unit test, integration, and/or e2e tests
  • Tested locally
  • Updated documentation
  • Update Swagger spec if the PR introduce API changes
  • Regenerated Golang and Python client if the PR introduces API changes

Notes for Reviewer

The version of MLP used here has a validation bug (which is updated on MLP#117). The code could still work with workaround (e.g. set FinalResponse: true in one async webhook if user use all async webhook, but it will be confusing for user since async webhook response is expected to not be used anywhere), so preferably to merge this PR after updating the MLP version as dependencies. (MLP version will be updated with the s3 PR)

Release Notes

add webhook event call if there's changes on model, model endpoint, model version

@bthari bthari added the enhancement New feature or request label Oct 7, 2024
Copy link

codecov bot commented Oct 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.68%. Comparing base (d71a53a) to head (8df6574).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #612      +/-   ##
==========================================
- Coverage   60.77%   60.68%   -0.09%     
==========================================
  Files         276      277       +1     
  Lines       22205    22245      +40     
==========================================
+ Hits        13494    13500       +6     
- Misses       7847     7877      +30     
- Partials      864      868       +4     
Flag Coverage Δ
api-test 58.66% <ø> (-0.09%) ⬇️
sdk-test-3.10 75.51% <ø> (ø)
sdk-test-3.8 75.49% <ø> (ø)
sdk-test-3.9 75.49% <ø> (ø)

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.

@bthari bthari marked this pull request as ready for review October 8, 2024 03:26
api/webhook/webhook.go Outdated Show resolved Hide resolved
api/api/model_endpoints_api.go Outdated Show resolved Hide resolved
api/queue/work/model_service_deployment_test.go Outdated Show resolved Hide resolved
api/queue/work/model_service_deployment_test.go Outdated Show resolved Hide resolved
@bthari bthari force-pushed the add-event-webhook branch from e097e56 to 8df6574 Compare October 11, 2024 10:14
Copy link
Contributor

@tiopramayudi tiopramayudi left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @bthari !

@bthari
Copy link
Contributor Author

bthari commented Oct 15, 2024

Thank you for the review and feedback @tiopramayudi @bayu-aditya!

@bthari bthari merged commit 0877a4f into main Oct 15, 2024
34 of 35 checks passed
@bthari bthari deleted the add-event-webhook branch October 15, 2024 06:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants