Add Valkey module#906
Conversation
✅ Deploy Preview for testcontainers-node ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| ]); | ||
| } | ||
| const startedContainer = new StartedValkeyContainer(await super.start(), this.password); | ||
| if (this.initialImportScriptFile) await this.importInitialData(startedContainer); |
There was a problem hiding this comment.
I know that this is copied from the Redis container code, but this can be refactored by moving this line into the containerStarted callback (https://node.testcontainers.org/features/containers/#lifecycle-callbacks). Extra ❤️ if you update the Redis container code as well 😄
There was a problem hiding this comment.
Will work for ❤️s.
Sure thing, I'll fix them both up
cristianrgreco
added
enhancement
cristianrgreco
changed the title
…key and redis containers
|
@cristianrgreco changes are ready for review. Thanks fro the input |
|
Thanks @mdodsworth. I had a thought. If Valkey is a drop-in replacement for Redis then could we get away with this? class ValkeyContainer extends RedisContainer {
constructor(image = "valkey/valkey:8.0") {
super(image);
}
// maybe need to override the start method to return a `StartedValkeyContainer`,
// but even this just calls `super(...)` ?
}EDIT: I think it may complicate things in terms of how the modules depend on each other. |
Yeah, my feeling was that the functionality may eventually diverge, so having it be separate container would be worthwhile |
2 participants
…ntainer
Adding support for Valkey. Container and tests are based largely on the existing redis setup.