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

WebUI: update users, spawn_rate, host and run_time in parsed_options (for LoadShapes that might access it) #2656

Merged
merged 2 commits into from
May 8, 2024

Conversation

raulparada
Copy link
Contributor

Description

Hit a bug where, by using a custom LoadTestShape with use_common_options (as described here) noticed that the run_time set through the web UI is not persisted to parsed_options (which I'd be accessing from my custom shape class' self.runner.environment.parsed_options.run_time). This value was thus always grabbing the initial value set through config / cli options, which is pretty unexpected to me. I understand the intended use case here, as specified in the docs, is to "reuse command line parameters", but since the web UI still allows changing this value updates should IMO reflect everywhere they're referenced downstream.

Changes

Persist this value to parsed_options_dict on the swarm endpoint handler. It'd make even more sense to me to do this for all values though, by changing the elif condition on current line 235 to if, not sure why it's done that way and why not all values are persisted/propagated.

Thanks!

@cyberw
Copy link
Collaborator

cyberw commented Mar 27, 2024

Does changing it from elif to if break any of the tests? If not, then I think that is preferable, like you say. Theres's probably some historic reason (not wanting to risk changing something when we introduced the ability to set extra web ui params)

@raulparada
Copy link
Contributor Author

When changing elif -> if on L235 two tests fail:

======================================== short test summary info ========================================
FAILED locust/test/test_web.py::TestWebUI::test_swarm_updates_parsed_options_when_multiple_userclasses_specified - AssertionError: Lists differ: ['U', 's', 'e', 'r', '1'] != ['User1', 'User2']
FAILED locust/test/test_web.py::TestWebUI::test_swarm_updates_parsed_options_when_single_userclass_specified - AssertionError: Lists differ: ['U', 's', 'e', 'r', '1'] != ['User1']

luckily written on #2201. And it makes sense actually, as would have to handle the parsing of each option (run_time for instance that I'm interested in needs parse_timespan). So it needs a bigger change. I also think these options should be exposed on a different place than environment.parsed_options, i.e. leave these read-only, and have another attribute with actual runtime values at any moment easily accessible on environment. I'm just getting started with the lib so not feeling confident enough about any of this though 😄

As for my PR as is, well, noticed that the same issue exists for parsed_optionss .num_users and .spawn_rate, so... not sure on the value of my single change, it even makes it more confusing updating only a few values.

@cyberw
Copy link
Collaborator

cyberw commented Mar 27, 2024

I think it is only user_classes that needs special treatment. What if you do it similar to this:

            if key == "user_classes": # need special treatment because it isnt a plain value
                 parsed_options_dict[key] = request.form.getlist("user_classes")
            elif key in parsed_options_dict:
                parsed_options_value = parsed_options_dict[key]
                ...

@cyberw
Copy link
Collaborator

cyberw commented May 2, 2024

@raulparada Do you think you'll have time to finish this?

@tiadobatima
Copy link

I'm quite interested in a solution for this too... In our environment, not everyone has access to touch the locust code and rerun with new params, but they have access to the UI and we want them to use our custom shapes. Cheers!

So they can be accessed from LoadTestShapes
@cyberw cyberw merged commit f33275d into locustio:master May 8, 2024
10 of 11 checks passed
@cyberw
Copy link
Collaborator

cyberw commented May 8, 2024

I took matters into my own hands :)

@cyberw cyberw changed the title fix: update parsed_options run_time WebUI: update users, spawn_rate, host and run_time in parsed_options (for LoadShapes that might access these) May 8, 2024
@cyberw cyberw changed the title WebUI: update users, spawn_rate, host and run_time in parsed_options (for LoadShapes that might access these) WebUI: update users, spawn_rate, host and run_time in parsed_options (for LoadShapes that might access it) May 8, 2024
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.

3 participants