-
Notifications
You must be signed in to change notification settings - Fork 162
feat: add redis pubsub support to EDFS #1810
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
…and-nats-as-datasource-in-router # Conflicts: # router/core/plan_generator.go
…ka, streamline provider initialization
…-datasource-in-router
…-datasource-in-router
…and planner files, enhance NATS subject validation
…actor-kafka-and-nats-as-datasource-in-router
…-datasource-in-router
…ment - Added InstanceData struct to encapsulate hostName and listenAddress. - Updated ExecutorConfigurationBuilder and ExecutorBuildOptions to include InstanceData. - Modified Loader and DefaultFactoryResolver to utilize InstanceData for better clarity and maintainability. - Refactored graphServer to initialize with InstanceData, enhancing the overall structure of the configuration.
…-datasource-in-router
…-datasource-in-router
…er to add a PubSubDataSource
…re up and also after they are stopped/started
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 Redis Pub/Sub support to the EDFS router and composition layers.
- Introduces Redis event provider in router configuration, test environment, and Docker compose.
- Extends protocol definitions, TypeScript bindings, and composition normalization to handle
edfs__redisPublish
/edfs__redisSubscribe
directives. - Updates CI workflow to include Redis-specific tests (pending file additions).
Reviewed Changes
Copilot reviewed 46 out of 46 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
router/debug.config.yaml | Example Redis pub/sub configuration added |
router/.mockery.yml | Added Redis adapter interface to mockery config |
router-tests/testenv/testenv.go | New flags, hosts and embed for Redis in testenv |
proto/wg/cosmo/node/v1/node.proto | Defined RedisEventConfiguration message |
connect/src/wg/cosmo/node/v1/node_pb.ts | Generated TS class for RedisEventConfiguration |
composition/src/v1/utils/string-constants.ts | Added Redis directive names and channel keys |
composition/src/v1/utils/constants.ts | Defined directive definitions for Redis publish/subscribe |
composition/src/v1/normalization/utils.ts | Registered Redis directive data |
composition/src/v1/normalization/normalization-factory.ts | Implemented Redis publish/subscribe handlers |
composition/src/v1/normalization/directive-definition-data.ts | Populated Redis directive definition data |
composition/src/router-configuration/types.ts | Extended EventConfiguration to include Redis |
docker-compose.yml | Healthchecks and restart policy for Redis added |
.github/workflows/router-ci.yaml | CI matrix updated to run Redis event tests |
Comments suppressed due to low confidence (4)
router-tests/testenv/testenv.go:325
- [nitpick] The new
EnableRedis
andEnableRedisCluster
flags inConfig
lack comments. Add doc comments to explain their purpose and usage in tests.
EnableRedis bool
router-tests/testenv/testenv.go:84
- The embedded file
testdata/configWithEdfsRedis.json
is referenced but not added to the repository. Either include this file or remove/update the//go:embed
directive.
//go:embed testdata/configWithEdfsRedis.json
.github/workflows/router-ci.yaml:141
- The CI workflow references
events/redis_events_test.go
, but this file does not exist. Update the workflow to point to the correct test path or add the missing test file.
test_target: ["./. ./fuzzquery ./lifecycle ./modules", "./telemetry", "./events/events_config_test.go", "./events/nats_events_test.go", "./events/kafka_events_test.go", "./events/redis_events_test.go"]
composition/src/v1/normalization/normalization-factory.ts:2467
- [nitpick] There's a
TODO
comment about list coercion. Consider implementing proper list value coercion or removing the TODO if it's no longer needed.
//@TODO list coercion
…e and other small things suggested by testifylint
…redis-pubsub-in-edfs
…redis-pubsub-in-edfs
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.
LGTM
Motivation and Context
Add Redis Pub/Sub adapter, depends on approval of #1848
Checklist