-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Work towards 1.0. Refactoring of runners/events/web ui. Getting rid of global state. #1266
Conversation
…e and RequestStats instance, by introducing Environment class that ties things together. Add WebUI class that creates the Flask app the web UI. Rename --port command line argument to --web-port (since we already use --web-host) Rename LocustRunner.start_hatching to start.
…ch_rate as an argument and there’s no need to store it on the runner instance)
…parse or mock options when using Locust as a lib)
…o not have to parse or mock options when using Locust as a lib)
… whole Environment
Codecov Report
@@ Coverage Diff @@
## master #1266 +/- ##
=========================================
+ Coverage 77.52% 80.12% +2.6%
=========================================
Files 20 23 +3
Lines 1984 2068 +84
Branches 313 319 +6
=========================================
+ Hits 1538 1657 +119
+ Misses 368 333 -35
Partials 78 78
Continue to review full report at Codecov.
|
Cool stuff! I’ll have a deeper look soon. |
btw, I like all the suggested "more changes" (though I think most of them of deserve their own PRs instead of having a mega-PR :) |
…nce it’s default has been set to True in msgpack 1.0. Fixes a crash bug when running Locust distributed with latest msgpack.
Python 3’s round() returns ints, so no need to call int(round()) any more.
You're right. We should probably branch of master in a 0.X branch, which we can use for bug fixes in the 0.14 version until we're ready to release a 1.0. Alternatively, we could make separate pull requests to this PR's branch (at least I think that's possible on Github?), though it's probably better to keep the 1.0 changes in master, and make a separate branch for the 0.14 version. |
We can branch a emergency fix branch for 0.14 if and only if we need it. I think GH will have an easier time keeping track of PRs & stuff if everything is merged to master instead of having PRs to this branch. But either way is fine. |
Sounds good! |
Below is a working example where Locust is used as a lib instead of started through the default command (main.py). Hopefully it's easier to grasp the changes to the internal API by looking at this example, compared to looking at all the changes in the PR. import gevent
from locust import HttpLocust, TaskSet, task, between
from locust.runners import LocalLocustRunner
from locust.env import Environment
from locust.stats import stats_printer
from locust.log import setup_logging
from locust.web import WebUI
setup_logging("INFO", None)
class User(HttpLocust):
wait_time = between(1, 3)
host = "https://docs.locust.io"
class task_set(TaskSet):
@task
def my_task(self):
self.client.get("/")
@task
def task_404(self):
self.client.get("/non-existing-path")
# setup Environment and Runner
env = Environment()
runner = LocalLocustRunner(environment=env, locust_classes=[User])
# start a WebUI instance
web_ui = WebUI(environment=env, runner=runner)
gevent.spawn(lambda: web_ui.start("127.0.0.1", 8089))
# start a greenlet that periodically outputs the current stats
gevent.spawn(stats_printer(env.stats))
# start the test
runner.start(1, hatch_rate=10)
# wait for the greenlets (indefinitely)
runner.greenlet.join() |
Cool stuff! Lets add a unit test that does something like that! |
… only using add_listener() and remove_listener()
…se EventHook.add_listener as a decorator
…the list of listeners
… code that is dependent on the Environment instance (e.g. to set up request_success/request_failure listeners). Update examples/events.py to use the new API.
After some discussion with @cyberw on Slack I've now reworked the API slightly.
|
…ll have module level events (locust.events.init)
Add init_command_line_parser event that can be used by test scripts and plugins to add command line arguments.
… the init event to set up event listeners
Introduce global instance of Events() stored in locust.events which can be used to register event listeners at the module level of locustfiles. Remove some unused imports.
Rename locust.events module to locust.event Introduce global instance of Events() stored in locust.events which can be used to register event listeners at the module level of locustfiles.
…tions with constants
…ers' dependency on parsed command line options)
Remove seemingly unnecessary sleep call.
…d, as it seems much too flaky
… Environment. Now they are just keyword arguments to MasterLocustRunner/SlaveLocustRunner.
Maybe we should rename this PR (because the name is not descriptive of the changes made) and try to merge before it grows too big? |
Yes, that sounds like a good idea. Though we should branch off the master into a 0.x branch (that we can use for critical bug fixes) before merging this. |
This has now been merged to master. I've created a new branch (v0.x) from the previous master. I've created a 1.0 milestone, and created stub issues for some of the items in my checkbox list under "More changed" in the OP. |
locust/clients.py
Outdated
# copy data from response to this object | ||
self.__dict__ = response.__dict__ | ||
self.locust_environment = environment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it could be called just ”environment” instead of locust_environment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefixed it with locust_
just to make sure that we won't clash with any (current or future) attribute in requests.Session.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yea, this is gone now anyways :)
locust/test/test_locust_class.py
Outdated
@@ -256,8 +258,8 @@ class MyLocust2(Locust): | |||
host = "http://127.0.0.1" | |||
task_set = MyTaskSet2 | |||
|
|||
l = MyLocust() | |||
l2 = MyLocust2() | |||
l = MyLocust(Environment(locust_classes=[MyLocust])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. An Environment has a list of Locust classes, but a Locust requires an Environment to be instantiated? I am not sure what the solution is, but this is a typical sign that there is too tight coupling between Env/Locust.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, maybe. Not sure though. One would never instantiate a Locust class like that except for in a test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is one of those things we spend a lot of time discussing later, so it is kind of old news :)
""" | ||
Simple, sample XML RPC client implementation that wraps xmlrpclib.ServerProxy and | ||
fires locust events on request_success and request_failure, so that all requests | ||
gets tracked in locust's statistics. | ||
""" | ||
|
||
_locust_environment = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe just call this _environment? Not that it is super important...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just wanted to make sure that we wouldn't clash with anything in xmlrpclib.ServerProxy
.
Hey!
I've done some refactoring of Locust. The main purpose is to improve the code quality, and make some parts of Locust that is currently non public part of the public API.
It's now in a working state but since it's a few changes to the public API, as well as a lot of changes to the internal API, I think it would be a good idea to have these changes in a 1.0. Therefore - if others doesn't think it's a terrible idea - I suggest that we call a feature freeze for master (and implement all new features in the v1.0 branch) since it'll be a messy job to port new features from master.
Please note that it's a work in progress. All tests pass, but there are no documentation changes yet. I wanted to get it out in a Github PR as soon as possible in order to get more eyes on it.
Implemented changes
These are the changes that are currently implemented in this branch:
Environment
class has been introduced to work as a glue between the runner, locust classes, events and web UI. The main benefit of this is that we can get rid of the globallocust.stats.global_stats
andlocust.runner.locust_runner
instances. End users will also be able to run Locust as a lib/programatically (How to run test programatically #222), and it'll be easier to write (unit) tests for Locust.locust.events
has been moved to theevents
attribute of theEnvironment
instance.Locust
classes must now be passed anEnvironment
instance reference (when instantiated) which is then stored onLocust.environment
. It's then used by the HTTP client when it triggersrequest_success
/request_failure
events.WebUI
class has been added in order to be able to spawn one (or multiple) web UI server(s) when running Locust programatically.LocustRunner.start_hatching()
has been renamed toLocustRunner.start()
LocustRunner.hatch_rate
has been removed in favour of it just being an argument tostart()
andspawn_locusts()
.More changes
Here are other changes that I think we should implement/consider for a 1.0. Most of them have been suggested/requested by others before at some point:
to figure outan API for some initialization hook in end users' test scripts. Since the the test script is imported before theEnvironment
is constructed, we can't rely on letting end users put the code at the module level.API:
Locust
toUser
(see Rename and restructure Locust/TaskSet #1264 for a longer discussion)Locust.setup
andLocust.teardown
hooks in favour of addingtest_start
andtest_stop
events toEnvironment.events
(Changes made during setup only effect the first user #1265).@seq_task
and instead add aSequentialTaskSet
class that run all declared tasks sequentially.host
tobase_url
(Build URLs using urljoin #1217)?These are the changes that I could think of from the top of my head right now. It's just a start and we can edit this post to keep the above list up to date.