WIP: Support RedisBacked Distributor#8403
Conversation
raju249
force-pushed
the
distributor-state-redis
branch
3 times, most recently
from
55d51e8 to
2497654
Compare
| String clazz = config.get(DISTRIBUTOR_SECTION, "implementation").orElse(DEFAULT_DISTRIBUTOR_SERVER); | ||
| LOG.info("Creating distributor server: " + clazz); | ||
| try { | ||
| Class<?> busClazz = Class.forName(clazz); |
There was a problem hiding this comment.
bus? Your copy-and-paste is showing :)
java/server/src/org/openqa/selenium/grid/distributor/redis/RedisDistributorFlags.java
Show resolved
Hide resolved
raju249
force-pushed
the
distributor-state-redis
branch
from
5e79877 to
463d67e
Compare
raju249
force-pushed
the
distributor-state-redis
branch
from
1eec14c to
e4816dd
Compare
shs96c
left a comment
shs96c
left a comment
There was a problem hiding this comment.
It's a good start. Don't be afraid to create a new package if you need to for the bits that are shared between multiple redis-backed implementations.
| "//java/server/test/org/openqa/selenium/grid:__subpackages__", | ||
| ], | ||
| exports = [ | ||
| "//java/server/src/org/openqa/selenium/grid", |
There was a problem hiding this comment.
Technically this should also be in deps. I'm mildly surprised this compiles (if it does!)
| @Override | ||
| public CreateSessionResponse newSession(HttpRequest request) | ||
| throws SessionNotCreatedException { | ||
| return null; |
| Require.nonNull("Node to add", node); | ||
|
|
||
| RedisCommands<String, String> commands = connection.sync(); | ||
| commands.mset( |
There was a problem hiding this comment.
Do we need the intermediate commands variable? Or just chain the method calls? Probably fine either way....
There was a problem hiding this comment.
Thinking to keep it the same 😅
| private String nodeUriKey(UUID id) { | ||
| Require.nonNull("Node UUID", id); | ||
|
|
||
| return "node:" + id.toString() + ":uri"; |
There was a problem hiding this comment.
id will be coerced to a string anyway, so the call to toString() can be omitted.
java/server/src/org/openqa/selenium/grid/distributor/redis/RedisDistributorFlags.java
Show resolved
Hide resolved
| private final Config config; | ||
|
|
||
| public RedisDistributorOptions(Config config) { | ||
| this.config = config; |
raju249
force-pushed
the
distributor-state-redis
branch
from
b1d2200 to
84901b3
Compare
|
Thank you @raju249 for your contribution! Appreciate the work. It has definitely helped as a boilerplate to get started. However, since the last contribution to this branch, the Distributor and Grid, in general, has gone through a bunch of enhancements ever since. Based on the changes made here, Redis Distributor is introduced as part of 192aed6 (other commits are also involved but the above mentioned is the crux). |
5 participants
Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.
Description
Adds support for Distributor backed with redis for storing info about the nodes in the grid.
Motivation and Context
Types of changes
Checklist