AOR-106: Add Webhook class and WebhookProvider to bridge-sdk#18
AOR-106: Add Webhook class and WebhookProvider to bridge-sdk#18freddie-poolsideai wants to merge 12 commits intomainfrom
Conversation
Introduces Webhook model (name, provider, filter_expression, transform_expression, idempotency_key_expression) and WebhookProvider constants. Adds webhooks field to Pipeline and PipelineData, and passes them through in the DSL output.
Tests Webhook model, WebhookProvider constants, Pipeline with webhooks, PipelineData serialization, DSL output format, and repr.
Adds Webhooks section documenting Webhook class, WebhookProvider constants, CEL expressions, and available providers.
…ion to filter/transform/idempotency_key Shorter kwarg names to match the RFC. Added model_validator to reject idempotency_key for non-generic HMAC providers.
Generic providers (generic_*) now require idempotency_key. Named providers
reject it. Validator uses startswith('generic_') for future extensibility.
freddie-poolsideai
requested review from
agoddijn-ps,
andreibratu and
tkpoolside
| filter='payload.type == "Issue" && payload.action == "update"', | ||
| name="linear-issues", | ||
| provider=WebhookProvider.LINEAR, | ||
| transform='{"triage_step": {"issue": payload.data}}', |
There was a problem hiding this comment.
I might lack the bridge knowledge - it this flexible enough to allow starting from an arbitrary point in the DAG? If so, will it use cached inputs?
There was a problem hiding this comment.
At the moment, no. It's only for an entire pipeline run. I think to run from arbitrary points we'd later add step entry points which would support this. I don't want us to increase scope by adding this now though.
| name="on_issue_update", | ||
| webhooks=[ | ||
| Webhook( | ||
| branch="main", |
There was a problem hiding this comment.
The branch= approach could spell trouble down the line, but I'm aware it's difficult to get around.
My concerns are:
- It introduces an external dependency on git which is easy to miss e.g. "why are my hooks not firing on the DEV branch, oh
branch=main" - It bakes in the assumption that every pipeline has one "main" branch and all other versions are "wip" branches. I don't think it's unrealistic for users to have alternative workflows of the same feature for business logic e.g. "handle_customer_complaint_us", "handle_customer_complaint_gdpr".
I think we should either allow
- Turning webhooks on/ off in the UI of bridge per branch
- If we want to keep all configuration in Git, allow CEL expressions or regexes for
branch=. Implementation should allow execution on multiple branches (yet also restrict to one branch at a time? Will think about it)
There was a problem hiding this comment.
The branch here specifically refers to when we'll index the webhook. We don't want webhooks for every branch I don't think, this will be confusing for customers. It'll also lead to issues whereby a webhook (for main for example) exists on multiple branches - which do we index and take as the current configuration? This is why we require branch="main" or similar as this declares only index this on a given branch. Arguably we could hit trouble if a user declares webhooks with the same name on different branches, but this isn't a problem as designed.
We will show the branches that each webhook corresponds to in the UI, so customers will be able to see what webhooks do I have on each branch. I think this mitigates 1.
I don't think 2 is true. You can have multiple webhooks on whichever branches you choose. If you have multiple branches that should have the same webhook configuration (albeit on different endpoints) you could use a for loop to achieve this with the required branch names.
- We will support enabling / disabling in Bridge, this is part of the design already.
- This is an idea, and it is more flexible, but I'd like to start with magic strings. We can add this in, but it's more difficult to take away.
tkpoolside
left a comment
tkpoolside
left a comment
There was a problem hiding this comment.
Let's update README and add some examples
Add Webhooks section to README covering named and generic providers, CEL filter/transform/idempotency_key expressions, and provider list. Update API Reference with Webhook and WebhookProvider imports. Add two example files: - webhook_example.py: Linear and GitHub triggers with agent triage - webhook_generic_example.py: generic HMAC provider with idempotency key
Vary branch values across webhooks (main, production, staging) to show that branch controls which version of the pipeline code runs. Add inline comments explaining the branch field in examples, README, and SKILL.md.
The webhook branch field determines where the webhook is discovered (indexed) from and which version of the pipeline code runs when it fires. Previous wording only mentioned the execution side.
3 participants
Summary
WebhookPydantic model withname,provider,filter_expression,transform_expression, and optionalidempotency_key_expressionWebhookProviderconstants for supported providers (github, gitlab, grafana, linear, slack, stripe, generic_hmac_sha1, generic_hmac_sha256)webhooksfield toPipelineandPipelineData, included in DSL outputWebhookandWebhookProviderfrombridge_sdkTest plan
uv run pytest -v)Closes AOR-106