-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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
Move node selection logic in a package #8513
Conversation
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.
@raju249
Could you please add some context to the PR? The PR template is being completely ignored.
How can this be tested? Why are these changes made?
Hey @diemol I have updated the PR description. Please take a look. Thanks 🙇 |
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.
I suspect that what we want to do is to ensure that the model is immutable, and a snapshot of the understood state of the system at the time it's created. It's also just a model, so there's no need to have things like Host.runHealthCheck
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.
@raju249 I made the changes we spoke about. Please have a look, and if you are ok with them, we can merge this. Thanks!
Thanks much @diemol 🙇 Changes LGTM. 👍 We can land this. |
Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it
Description
This PR separates the tightly coupled logic of
nodeSelection
to run a session, into a separate package and can be independently modified going forward. It doesn't and should not change any functionality from previoud build and should not show any differences for the user to run a session.Motivation and Context
RedisDistributor
for which this same nodeSelection Package would be used/modified to make it work with redis backed distributor.RedisDistributor
PR, this PR will make that change and I would iterate from there after it is merged.GridModel
class stores the nodes and listens to addition and removal of Nodes. TheNodeSelector
class will help us select the node from the pool of nodes. Now, this pool of the nodes can be an in-memory hash map for a local distributor, it can also be a redis backed store forRedisBasedDistributor
or a database backed store forJdbcBackedDistributor
The changes to make the datastore configurable inside the
NodeSelector
class will be done in #8403Types of changes
Checklist