setup() should always run after __init__()

pszynk commented 6 years ago

Description of issue / feature request

In the current implementation Locust.setup() method is run from within Locust.__init__(). But what if you want to inherit from class Locust add your own __init__() method and also use setup(). The most proper way would be:

class MyLocust(Locust):
  def __init__(self):
     # you can call it last I guess 
     super(MyLocust, self).__init__()
     self.some_var = os.getenv("SOME_VAR")

  def setup(self):
    # self.some_var not yet initialized
    self.client = Client(self.some_var)

Expected behavior

I think that the best way would be to assure that setup() always runs after __init__(). Of course you can put whole initialization inside setup() or just call super().__init__() last, but that probably needs documentation.

aldenpeterson-wf commented 6 years ago

Why not just rename your method:

class MyLocust(Locust):
  def __init__(self):
     # you can call it last I guess 
     super(MyLocust, self).__init__()
     self.some_var = os.getenv("SOME_VAR")
     self.custom_setup()

  def custom_setup(self):
    self.client = Client(self.some_var)

pszynk commented 6 years ago

Well, setup() guarantees that it'll run only once.

Either way, I wasn't fully aware how the threads are initiated and how setup() is called. Now I see that setting self.client in setup() is stupid because it will run only for the first thread. So setting anything for Locust instance in setup() is rather pointless. If it's instance independent (maybe I'm wrong) wouldn't it make more sense to make setup() a @classmethod ?

cgoldberg commented 6 years ago

So setting anything for Locust instance in setup() is rather pointless

Not pointless, just used for different setup tasks. I think what you are looking for are the on_start/on_stop methods. They differ from setup/teardown as they are run per TaskSet instance.

have you read the new docs?... they explain it well: https://docs.locust.io/en/latest/writing-a-locustfile.html#setups-teardowns-on-start-and-on-stop

setup/teardown:

"setup and teardown, whether it’s run on Locust or TaskSet, are methods that are run only once. setup is run before tasks start running, whileteardown is run after all tasks have finished and Locust is exiting. This enables you to perform some preparation before tasks start running (like creating a database) and to clean up before the Locust quits (like deleting the database)."

vs.

on_start/on_stop:

"A TaskSet class can declare an on_start method or an on_stop method. The on_start method is called when a simulated user starts executing that TaskSet class, while the on_stop method is called when the TaskSet is stopped."

note: setups/teardowns are a new feature that have yet to be released (though they exist in master)... so feedback on the implementation, usage, and documentation is very welcome.

pszynk commented 6 years ago

Ok, I see that I was wrong, setup() should NOT execute after __init__(). In fact now I think it should execute before any instance is initialized.

I guess my confusion, as a person who just started using your package, comes from not knowing which code is run by multiple threads. You got Locust and TaskSet. Locust represents the user and each user has its own thread running the code of user's TaskSets. Am I wrong?

1. `Locust.setup()` and `Locust.teardown()`

Runs only once no matter how many user threads are spawned.

2. `TaskSet.setup()` and `TaskSet.teardown()`

Runs only once no matter if a given TaskSet is used in multiple Locusts and how many user threads are spawned.

3. `TaskSet.on_start()` and `TaskSet.on_stop()`

Runs only once per user thread? This is where I'm a bit confused.

The on_start method is called when a simulated user starts executing that TaskSet class, while the on_stop method is called when the TaskSet is stopped."

User can have multiple TaskSets but when you say

stars executing

the TaskSet is started only once for each user?

Conclusion

Of course setup()/teardown() and on_start()/on_stop() are very useful features and in no way I'm saying that any of them is pointless. What I was looking for is some way to load a configuration passed to given locust execution (in this case by env variables, but it could be a config file as well). There is no need to load the configuration multiple times, so I thought that Locust.setup() would be the right place to do it. The problem is that Locust.setup() should not be used to modify given Locust instance in any way. What I could do is pass configuration to Locust class, e.g.

class BadSetupLocust(Locust):
  # This will execute only for the first instance of BadInitLocust 
  # (first time BadInitLocust.__init__() is run)
  def setup(self):
     self.some_var = os.getenv("SOME_VAR")

class BetterSetupLocust(Locust):
  # This will execute only for the first instance but will 
  # initiate class variable for all instances
  def setup(self):
     BetterInitLocust.some_var = os.getenv("SOME_VAR")

class GoodSetupLocust(Locust):
  # like the previous one, and there is no way to do sth 
  # confusing - like configure only the first initiated instance
  @classmethod
  def setup(cls):
     cls.some_var = os.getenv("SOME_VAR")

My problem is that I see no good place for loading and applying configuration. I've put configuration loading in Locust.__init__(), it works but the code executes for each thread and there is no need for that because users share the configuration.

Good job on your project btw, it's a really nice tool.

aldenpeterson-wf commented 6 years ago

Sorry for the late response @pszynk but I think you can do what you are trying to do simply by including the config in the locustfile itself, run as part of the file, because that will only be run once when it's included.

Something like:

some_var = os.getenv("SOME_VAR")
 # only will see once regardless of how many clients you simulate
print('Got some_var from the env:{}'.format(some_var)

class GoodSetupLocust(Locust):
 @classmethod
  def setup(cls):
     global some_var
     cls.some_var = some_var

Presumably your setup is more complicated and it may require a slightly different init here but an approach like this should solve your "how do I load config once?" question.

locustio / locust