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

Handle task weights #2741

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

bakhtos
Copy link
Contributor

@bakhtos bakhtos commented May 27, 2024

Fixes #2651

tasks attribute of TaskSet/User is now a mapping from tasks to their weights.
All other utility functions, like tag, get_tasks_from_base_classes, filter_tasks_by_tags take this into account in their recursion.

gen_next_task now uses random.choices to sample the tasks according to weights. Necessary cumulative weights are pre-computed in __init__.

The check of whether there are no tasks is now performed at __init__, instead of doing it on every call to gen_next_task.

Additionally, the DefaultTaskSet class is removed because it was only necessary to override two functions where self.user should be used instead of self when used directly inside a User. User now uses a TaskSet internally, and TaskSet now checks if it is being used by the User or nested in another TaskSet to make the correct choice.

Currently, this is a draft PR because only the backend itself is changed. Please let me know if the changes are understandable so far, and then I will proceed to change the tests and documentation to reflect this.

@bakhtos bakhtos changed the title Feature/store task weights Handle task weights May 27, 2024
@bakhtos
Copy link
Contributor Author

bakhtos commented May 27, 2024

I see based on the failed tests already that if TaskSet is instantiated in User then the check of empty tasks in __init__ will raise an error. There is a simple fix for that, will add it soon

@cyberw
Copy link
Collaborator

cyberw commented May 27, 2024

Hi - looks like I reviewed your PRs out of order :)

I will do a full review once the build is green.

I do get some errors out of the box running for example examples/locustfile.py.

[2024-05-27 19:06:22,011] Larss-MacBook-Pro/INFO/locust.runners: Ramping to 1 users at a rate of 1.00 per second
[2024-05-27 19:06:22,012] Larss-MacBook-Pro/INFO/locust.runners: All users spawned: {"QuickstartUser": 1} (1 total users)
Traceback (most recent call last):
  File "src/gevent/greenlet.py", line 908, in gevent._gevent_cgreenlet.Greenlet.run
  File "/Users/lars/git/locust/locust/user/users.py", line 189, in run_user
    user.run()
  File "/Users/lars/git/locust/locust/user/users.py", line 147, in run
    self._taskset_instance = TaskSet(self)
                             ^^^^^^^^^^^^^
  File "/Users/lars/git/locust/locust/user/task.py", line 293, in __init__
    raise Exception(
Exception: No tasks defined on TaskSet. Use the @task decorator or set the 'tasks' attribute

Also DefaultTaskSet is used in other places (like locust-plugins), so maybe we leave an implementation there for a while and just mark it deprecated?

@cyberw
Copy link
Collaborator

cyberw commented May 27, 2024

Oh, and one more thing, would this change impact the "Current ratio" tab in the UI? (http://0.0.0.0:8089/?tab=ratios)

@bakhtos
Copy link
Contributor Author

bakhtos commented May 29, 2024

@cyberw

I will do a full review once the build is green.

Okay, then I will work further on fixing the tests. Will take some time.

Oh, and one more thing, would this change impact the "Current ratio" tab in the UI? (http://0.0.0.0:8089/?tab=ratios)

Does this get the data from this module? https://github.com/locustio/locust/blob/master/locust/user/inspectuser.py
If so, those functions are fixed internally to account for the dictionary of tasks to weights, but the interface and output should remain equivalent. If these are tested, once the build is fixed it should be ok.

@cyberw
Copy link
Collaborator

cyberw commented Jun 25, 2024

Hi @bakhtos have you had any time to look at this? :)

@bakhtos
Copy link
Contributor Author

bakhtos commented Jun 26, 2024

Hi @cyberw !

I started looking into fixing the tests for other PRs that I prepare here, and then I had a busy time at work.

Next week I am almost free from work, so I hope to finalize these PRs.

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.

Use random.choices when selecting a task instead of random.choice
2 participants