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: Implement webhooks library and perform webhook call on project event #96

Merged
merged 36 commits into from
Jun 5, 2024

Conversation

shydefoo
Copy link
Contributor

@shydefoo shydefoo commented May 14, 2024

Description

  • This PR introduces a new webhooks package. The package is meant to be used across caraml components (e.g. merlin, turing, mlp) to call webhooks when specific events occur.
  • The package contains the webhook client implementation and abstracts the logic from the user. It provides some helper functions for the user to call in their code when specific events occur.
  • The payload to the webhook server and the response can be arbitrary, and it is up to the user to choose what payload to send to the webhook server(s), but only 1 response will be used in the callback
  • The response from the webhook is also arbitrary, and it is the responsibility of the component to define/validate the structure of the response from the webhook.
  • See the package readme for a detailed explanation of how the webhooks can be configured.

How to use?

  1. In the caller package (eg, mlp, merlin), define the list of events that requires webhooks. For example:
const (
	ProjectCreatedEvent wh.EventType = "OnProjectCreated"
	ProjectUpdatedEvent wh.EventType = "OnProjectUpdated"
)

var EventList = []wh.EventType{
	ProjectCreatedEvent,
	ProjectUpdatedEvent,
}
  1. Define the event to webhook configuration. Optionally, the configuration can be provided in a yaml file and parsed via the Config struct. In the config file, define the event to webhook mapping for those events as required. For example, if projects need extra labels from an external source, we define the webhook config for the OnProjectCreated event
webhooks:
  enabled: true
  config:
    OnProjectCreated:
      - url: http://localhost:8081/project_created
        method: POST
        onError: abort
        finalResponse: true
  1. Call InitializeWebhooks() to get a WebhookManager instance.
    This method will initialize the webhook clients for each event type based on the mapping provided
projectsWebhookManager, err := webhooks.InitializeWebhooks(cfg.Webhooks, service.EventList)
  1. Call InvokeWebhooks() method in the caller code based on the event. For a more complete example, see here

Project Service changes

  • This PR contains 1 example showing how the webhook package can be used when a project is created
  • If there are subsequent steps to take, for example labels to add on the project, the webhook is invoked
  • A callback is passed to the webhook package to save the updated project data in the database.

Note for reviewers

  • More unit tests will be added after 1st round of discussion

@shydefoo shydefoo requested review from a team, deadlycoconuts, leonlnj, ariefrahmansyah and mbruner and removed request for a team May 15, 2024 05:19
@shydefoo shydefoo force-pushed the implement-webhooks branch from 11d67df to 137992f Compare May 16, 2024 07:25
@shydefoo shydefoo force-pushed the implement-webhooks branch from c14b1d0 to 368fd35 Compare May 17, 2024 05:10
@shydefoo
Copy link
Contributor Author

shydefoo commented Jun 4, 2024

@leonlnj thanks for the feedback, have addressed most of the changes, lmk what you think. Thanks!

Copy link

@leonlnj leonlnj left a comment

Choose a reason for hiding this comment

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

thanks, looks much better (at least for me) now!

Copy link
Contributor

@deadlycoconuts deadlycoconuts left a comment

Choose a reason for hiding this comment

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

Thanks for the new series of changes! 🚀 LGTM!

@deadlycoconuts
Copy link
Contributor

Ooops wait wait, there's one last thing - can you add the webhook calls to the UpdateProject service method? I think I mentioned this here but you might've left it out.

@shydefoo
Copy link
Contributor Author

shydefoo commented Jun 5, 2024

Ooops wait wait, there's one last thing - can you add the webhook calls to the UpdateProject service method? I think I mentioned this here but you might've left it out.

@deadlycoconuts added

@shydefoo
Copy link
Contributor Author

shydefoo commented Jun 5, 2024

Merging this now, thanks for the review @leonlnj and @deadlycoconuts

@shydefoo shydefoo merged commit 529cb48 into main Jun 5, 2024
9 of 10 checks passed
@shydefoo shydefoo deleted the implement-webhooks branch June 5, 2024 10:06
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