Skip to content
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

pluggable dispatcher logic #2606

Merged
merged 2 commits into from Feb 20, 2024
Merged

Conversation

mgor
Copy link
Contributor

@mgor mgor commented Feb 16, 2024

superseeding #2568, coming from discussion in #2522.

this PR makes it possible to implement custom dispatcher logic and specifying the dispatcher class when creating an Environment.

a little unsure and not really satisfied with the name of the "interface" UserDispatcherType.
played around with UserDispatcherBase (but it's discouragedto use Base). IUserDispatcher doesn't feel pythonic. so any suggestion on a better name is welcomed :) my top choice would be to rename UserDispatcher to WeightedUserDispatcher (or similar) and have the interface changed to UserDispatcher, but at the same time I wanted to make as few changes as possible.

some methods and properties in the interface was added to make it easier to [compability] test implementations.

so minor changes in the current dispatcher logic so it uses the interface specified methods and properties.

@cyberw
Copy link
Collaborator

cyberw commented Feb 19, 2024

Idk, this still changes/adds a lot of code....

Could you just subclass UsersDispatcher and override its methods insteas of introducing an abstract base class? (and not rename the methods. tbh I'm not sure about the reasoning for making them all private (_ prefix) but I'd rather not change it)

I understand it is not the cleanest OO-principles to do that, but low complexity in locust core is more important.

@mgor mgor marked this pull request as draft February 19, 2024 09:20
@mgor
Copy link
Contributor Author

mgor commented Feb 19, 2024

sure, update with the only change in locust.dispatch being some type annotations coming soon.

changed the PR to a draft, since I'd like to test them out in our project first, which uses the branch in my fork that this PR also is based on.

@mgor mgor marked this pull request as ready for review February 19, 2024 12:24
@cyberw cyberw merged commit cc5e373 into locustio:master Feb 20, 2024
15 checks passed
@cyberw
Copy link
Collaborator

cyberw commented Feb 20, 2024

nice!

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.

None yet

2 participants