Skip to content

Conversation

@tonyhb
Copy link
Contributor

@tonyhb tonyhb commented Apr 29, 2025

This implements a KV storage interface for the aggregate expression engine. This lets us provide a new PebbleDB instance into aggregate engines, meaning that pauses will be written to disk instead of stored in memory.

type ExpressionEvaluator func(ctx context.Context, e Evaluable, input map[string]any) (bool, error)

// EvaluableLoader returns one or more evaluable items given IDs.
type EvaluableLoader func(ctx context.Context, evaluableIDs ...uuid.UUID) ([]Evaluable, error)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

cleanup - this is unused.

//
// An AggregateEvaluator instance exists for every event name being matched.
type AggregateEvaluator interface {
type AggregateEvaluator[T Evaluable] interface {
Copy link
Contributor Author

@tonyhb tonyhb Apr 29, 2025

Choose a reason for hiding this comment

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

For the sake of the review, we must store a concrete type in the aggregator for marshalling to and from the concrete type in the KV storage. This requires generics.

// Attempt to unmarshal an empty byte slice, ensuring that we have
// a concrete type instead of an interface.
if _, err := kvopts.Unmarshal([]byte("{}")); err != nil {
panic(fmt.Sprintf("unable to make KV for aggregate evaluator without concrete type: %s", err))
Copy link
Contributor Author

@tonyhb tonyhb Apr 29, 2025

Choose a reason for hiding this comment

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

this allows us to fail fast in unit tests, but will never hit in our services — KVOpts will always pass in a KV constructor.

@tonyhb tonyhb merged commit 2c48500 into main Apr 29, 2025
2 checks passed
@tonyhb tonyhb deleted the feat/pebble branch April 29, 2025 14:43
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.

3 participants