-
Notifications
You must be signed in to change notification settings - Fork 250
Feat: OLAP Table for CEL Eval Failures #2012
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
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Pull Request Overview
Adds a new OLAP table and end‐to‐end plumbing to record CEL (Common Expression Language) evaluation failures.
- Introduce a new enum and partitioned
v1_cel_evaluation_failures
table with accompanying migration - Extend the trigger repository to collect failures and persist them via a new
StoreCELEvaluationFailures
method - Wire up message payloads and controllers to publish and consume CEL failure events into OLAP
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
sql/schema/v1-olap.sql | Define new v1_cel_evaluation_failures enum and table partitions |
pkg/repository/v1/trigger.go | Capture and return CEL evaluation failures in TriggerFromEvents |
pkg/repository/v1/sqlcv1/olap.sql.go | Update partition function and add StoreCELEvaluationFailures SQL |
pkg/repository/v1/sqlcv1/olap.sql | Add SQLC definitions for partitioning and store failures query |
pkg/repository/v1/sqlcv1/models.go | Add enum, null wrapper, and model struct for CEL failure source |
pkg/repository/v1/olap.go | Define CELEvaluationFailure type, interface, and implementation |
internal/services/shared/tasktypes/v1/olap.go | Define CELEvaluationFailures message payload |
internal/services/controllers/v1/task/controller.go | Publish CEL failures to message queue |
internal/services/controllers/v1/olap/controller.go | Consume CEL failures messages and persist to OLAP |
cmd/hatchet-migrate/migrate/migrations/20250716183217_v1_0_29.sql | Goose migration for failure table |
Comments suppressed due to low confidence (5)
pkg/repository/v1/sqlcv1/olap.sql.go:2346
- Field
Tenantid
should beTenantID
to follow Go's convention for initialisms (and update JSON tag to"tenant_id"
).
Tenantid pgtype.UUID `json:"tenantid"`
pkg/repository/v1/olap.go:236
- Exported interface method missing a doc comment; please add a comment explaining what
StoreCELEvaluationFailures
does and its failure conditions.
StoreCELEvaluationFailures(ctx context.Context, tenantId string, failures []CELEvaluationFailure) error
pkg/repository/v1/olap.go:1693
- New persistence logic for CEL failures lacks accompanying unit or integration tests; consider adding tests to validate that failures are correctly stored.
func (r *OLAPRepositoryImpl) StoreCELEvaluationFailures(ctx context.Context, tenantId string, failures []CELEvaluationFailure) error {
internal/services/shared/tasktypes/v1/olap.go:16
- Unresolved package alias
v1
; this should reference the actual import alias (e.g.,sqlcv1.CELEvaluationFailure
) or alias the package correctly.
Failures []v1.CELEvaluationFailure
internal/services/shared/tasktypes/v1/olap.go:19
- Function signature uses
v1.CELEvaluationFailure
but the package is imported assqlcv1
; update to[]sqlcv1.CELEvaluationFailure
or alias the import asv1
.
func CELEvaluationFailureMessage(tenantId string, failures []v1.CELEvaluationFailure) (*msgqueue.Message, error) {
56fdfda
to
3fef74e
Compare
@@ -25,6 +31,19 @@ func (c *V1CELService) V1CelDebug(ctx echo.Context, request gen.V1CelDebugReques | |||
), | |||
) | |||
|
|||
if err != nil { |
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.
Isn't the point of the debug endpoint to return a readable error to the user here? Why would we ingest this into the table?
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.
🤦 yeah I guess no need, will remove this one
Co-authored-by: Copilot <[email protected]>
* feat: migration + go gen * feat: non unique source name * feat: api types * fix: rm cruft * feat: initial api for webhooks * feat: handle encryption of incoming keys * fix: nil pointer errors * fix: import * feat: add endpoint for incoming webhooks * fix: naming * feat: start wiring up basic auth * feat: wire up cel event parsing * feat: implement authentication * fix: hack for plain text content * feat: add source to enum * feat: add source name enum * feat: db source name enum fix * fix: use source name enums * feat: nest sources * feat: first pass at stripe * fix: clean up source name passing * fix: use unique name for webhook * feat: populator test * fix: null values * fix: ordering * fix: rm unnecessary index * fix: validation * feat: validation on create * fix: lint * fix: naming * feat: wire triggering webhook name through to events table * feat: cleanup + python gen + e2e test for basic auth * feat: query to insert webhook validation errors * refactor: auth handler * fix: naming * refactor: validation errors, part II * feat: wire up writes through olap * fix: linting, fallthrough case * fix: validation * feat: tests for failure cases for basic auth * feat: expand tests * fix: correctly return 404 out of task getter * chore: generated stuff * fix: rm cruft * fix: longer sleep * debug: print name + events to logs * feat: limit to N * feat: add limit env var * debug: ci test * fix: apply namespaces to keys * fix: namespacing, part ii * fix: sdk config * fix: handle prefixing * feat: handle partitioning logic * chore: gen * feat: add webhook limit * feat: wire up limits * fix: gen * fix: reverse order of generic fallthrough * fix: comment for potential unexpected behavior * fix: add check constraints, improve error handling * chore: gen * chore: gen * fix: improve naming * feat: scaffold webhooks page * feat: sidebar * feat: first pass at page * feat: improve feedback on UI * feat: initial work on create modal * feat: change default to basic * fix: openapi spec discriminated union * fix: go side * feat: start wiring up placeholders for stripe and github * feat: pre-populated fields for Stripe + Github * feat: add name section * feat: copy improvements, show URL * feat: UI cleanup * fix: check if tenant populator errors * feat: add comments * chore: gen again * fix: default name * fix: styling * fix: improve stripe header processing * feat: docs, part 1 * fix: lint * fix: migration order * feat: implement rate limit per-webhook * feat: comment * feat: clean up docs * chore: gen * fix: migration versions * fix: olap naming * fix: partitions * chore: gen * feat: store webhook cel eval failures properly * fix: pk order * fix: auth tweaks, move fetches out of populator * fix: pgtype.Text instead of string pointer
9bae816
to
e4aaa66
Compare
Description
Adding a table in the OLAP db for temporarily storing CEL evaluation failures + wiring up writes. We can also add this to the debug endpoint + webhooks once it merges
Will need to update the webhooks + debug endpoint PRs once this goes in
Type of change