Return the new users on Runner.spawn_users

[pappacena]

I'm using Locust as a library for my custom tests scenarios, and I need to identify my test users with authentication tokens that are received from another stress test system.

I had some trouble accomplishing that with the current structure, since we can't add extra parameter to user's __init__ when they are spawned. This PR addresses this.

Let me know if there is a better way to do that.

[cyberw]

I’m kind of hesitant to adding functionality that is only accessible when using locust as a library (not saying it has to be a bad idea, but it creates a bit of a niche use case)

Also, does your approach really work when running distributed?

[pappacena]

@cyberw, it works when running distributed in the sense that it doesn't break anything, but I'm using some custom messages to spawn the users in a distributed way (using a mechanism similar to the one proposed in #1782).

I totally understand your concern about creating a feature that is not widely used. Maybe we could just return the list of spawned users in runner.spawn_users, then?

This way we could at least track and modify the objects after they were created, which would make Locust more flexible when used as a library without introducing an obscure feature for the other users.

[cyberw]

I totally understand your concern about creating a feature that is not widely used. Maybe we could just return the list of spawned users in runner.spawn_users, then?

That sounds like a great idea. But wouldnt it run into the same issue of not really being supported in distributed?

Just to be clear, its not just about creating a feature that isnt widely used, my concern is more about creating a feature that only works in some situations (non-distributed)

[pappacena]

I'm changing only Runner.spawn_users method, which is quite internal. It's only called by [Local|Master|Worker]Runner.start method, and it seems that the start method is the one that is most commonly used (advised at use_as_lib.py, and called by the web UI endpoint).

Making Runner.spawn_users return the newly created objects is a way to better control the flow for the niche users that are using Locust as a lib dealing with its internals, but shouldn't change anything to everyone else (since [Local|Master|Worker]Runner.start method is not being changed here).

[cyberw]

Looks good now I think. Could you add some documentation? I'm guessing this only works for LocalRunner, not distributed, right? Mention that in the docs too.

[cyberw]

I think this may have conflicts with the big PR on correct user distribution #1621 and I think it is easier to fix any conflicts in this PR rather than the other way around so I think we'll need to pause it for now.

[mboutet]

@cyberw, I think (but I'm not sure) these changes could work with #1621. From what I understand of @pappacena's use case, they completely bypass the dispatch and distribution logic by manually controlling the spawned users instead of relying on the usual start method.

Also, I don't think this would work in distributed mode as the users are instantiated on the workers.

I'm having trouble visualising how they are accomplishing what they're doing. I'm wondering how and where they are passing those extra arguments as nothing extra is passed to new_user = user_class(self.environment) according to the changes.

Perhaps @pappacena could share more details or, better, share a minimal example code of how they use it?

Also, may I suggest using the locust.user.users.User.on_start method instead to fetch the token for each user? That would be, in my opinion, a more scalable way of doing it and would work in both local and distributed modes.

EDIT:
I just saw the extra args were being pass to the user in previous commits but this code is no longer there.

[pappacena]

@mboutet indeed, I've removed the extra arguments after the feedback from cyberw.

I'm bypassing the distribution logic, and controlling manually the user creation (both locally and distributed).

My scenario is a bit complex, and Locust is just part of a stress test solution. More specifically, my simulated users are doing authenticated HTTP requests, but the token is fetched by another stress test tool, and I'm synchronizing those tokens with Locust by extending Locust endpoints.

So, I'm manually controlling which players are spawn in which worker, and I'm using Locust master-worker messaging system to pass these tokens (and some other set of relevant information), spawn the users and set their token accordingly.

It would be complicated to use User.on_start in this scenario. I would need another entity in my architecture to keep a pool of tokens or some other complicated strategy. And when a new worker joins, it would be a bit messy to deal with the rebalancing too.

[cyberw]

If you resolve the conflicts and add documentation then I'll make it part of 2.1 (whenever that happens - it depends on the stability of 2.0 I guess :)

[pappacena]

@cyberw just pushed some documentation. Let me know if that is clear enough.

I've changed the PR title to better match the final result.

[cyberw]

👍 We’re reworking the user selection logic a little before 2.0 release version so it will be a while before we do 2.1. I’ll take another look at this then.

[cyberw]

I wonder if this still works with 2.0. Can you resolve the conflicts and we'll see?

[cyberw]

Hmm... I'm still a little sceptical to this. I think using messages makes more sense #1782

[cyberw]

Could you try using messages instead?

[pappacena]

As I mentioned before, using custom messages wouldn't prevent the rebalancing from kicking in, for example, right?

TBH, I don't understand which are the concerns about making Runner.spawn_users return the objects it was asked to create.

I mean, when using Locust as a library, what is the benefit of not having an easy way to access the User objects that Locust engine creates?

[cyberw]

My main concern is introducing a conceptual break between distributed and local running. I only closed it because I thought you didnt care because you hadnt responded in so long :)

[pappacena]

@cyberw maybe I could just avoid documenting it as a "feature"? This way, returning the objects would be just a good practice to allow Locust lib to be "open for extension and closed for modification", but not giving the impression that it's something that should be used by the end user in common situations.

An alternative approach would be extending the documentation to show how to intentionally skip entirely the rebalancing and use messages to control that, as I'm doing today. But I admit that this use case is probably not that common, and explaining this may be better suited for a blog post somewhere, rather than in the official documentation.

[cyberw]

Hmm.. Couldn't you just have the Users ask the master for a token (in the User's on_start, over custom messaging), and the master respond with it? I realize that is significantly more complex than the suggested solution for your particular use case, but I would much prefer a solution that is more generally useable.

The implementation is also a little sneaky, because if you sleep (or just do some non-blocking I/O), that would allow the User greenlets to run after the spawn(), before your code setting User properties gets executed.

I know your code change is very small, so it wouldnt hurt that much to add it, but I still think it would be misleading to our users.

[cyberw]

Or, we could just try to be pragmatic (I realized what a mess it would be too use messaging).

If you add documentation making it clear that this only works on standalone and is an experimental feature, and may be removed at some point I’m ok with merging (it probably wont be removed, unless there is a good replacement)

[cyberw]

Or, we could just try to be pragmatic (I realized what a mess it would be too use messaging).

If you add documentation making it clear that this only works on standalone and is an experimental feature, and may be removed at some point I’m ok with merging (it probably wont be removed, unless there is a good replacement)

@pappacena If you do those things (and resolve the conflicts) soon then it will be included in 2.2 release

[pappacena]

Hi, @cyberw ! I'll try to do it in some hours. Thanks!

[cyberw]

Nice. I'm ok with merging now, unless there is something more you want to do? I'll squash the commits into one (as the first ones were heading in a different direction).

[pappacena]

It seems to be ok now. Thanks for your feedbacks, @cyberw !