clean up core.py & __init__.py

[cyberw]

core.py is big and messy. I think we should:

• Remove NoClientWarningRaiser because I dont think it is needed any more? (at least the comment would suggest that)
• Move Locust & HttpLocust into separate file (maybe locusts.py?)
• Move TaskSet* into separate file (maybe tasks.py? maybe also move the contents of sequential_locust.py into this file?)

We can still export them in __init__.py, to keep the external interface unchanged.

Some things should probably be removed from __init__.py though:
from .exception import InterruptTaskSet, ResponseError, RescheduleTaskImmediately
and
from .wait_time import between, constant, constant_pacing (the documentation instructs the user to get them from locust.wait_time module anyway...)

If it lgty @heyman I can make a PR for this.

[heyman]

I think it's a good idea!

Remove NoClientWarningRaiser because I dont think it is needed any more? (at least the comment would suggest that)

It's still used by Locust.client, but could be removed. A long time ago the Locust class had an HTTP client, and when that was moved to HttpLocust, we created the warning class to be able to give a better error message if people inherited from Locust and expected there to be HTTP client on the client attribute.

• Move Locust & HttpLocust into separate file (maybe locusts.py?)
• Move TaskSet* into separate file (maybe tasks.py? maybe also move the contents of sequential_locust.py into this file?)

Yes, I think that's a good idea. I think we should still have those modules under a common core package though (since they are still closely related). I think this structure would make sense:

core/
  - __init__.py
  - task.py
    - get_tasks_from_base_classes  (needed by both LocustMeta and TaskSetMeta)
    - @task
    - TaskSetMeta
    - TaskSet
    - SequentialTaskSet
    - DefaultTaskSet
  - user.py
    - LocustMeta
    - Locust
    - HttpLocust

If we do such change, we should start with single commit that just moves core.py to core/task.py (or whatever naming we choose) so that we preserve git log and git blane functionality for as large portion of the code as possible.

Some things should probably be removed from init.py though:
from .exception import InterruptTaskSet, ResponseError, RescheduleTaskImmediately

👍

and
from .wait_time import between, constant, constant_pacing (the documentation instructs the user to get them from locust.wait_time module anyway...)

I think we should keep at least constant and between in the root namespace as a convenience shortcut. But I'm fine with having to import other wait time functions from other modules.

[cyberw]

cool, I'll get started! soon-ish anyway...

I think we should keep at least constant and between in the root namespace as a convenience shortcut. But I'm fine with having to import other wait time functions from other modules.

I dont quite agree on this. I think we should export all or none of the wait time functions from locust package. I'd prefer removing them (as things like editor auto completion would be much improved), but if I can't remove all of them then I think we should keep them all :)

[heyman]

I'd prefer removing them (as things like editor auto completion would be much improved)

Hm, why is that the case?

but if I can't remove all of them then I think we should keep them all :)

I'm fine with keeping all of them (maybe that's even better). Our API is fairly small so I don't think the locust.* namespace feels too crowded.

[heyman]

Hm, regarding the package/module structure I suggested. core feels a bit too generic. Since all of that code is related to users simulation, I think it would be better to call the package user with the modules task and users.

user/
  - __init__.py
  - task.py
    - get_tasks_from_base_classes  (needed by both LocustMeta and TaskSetMeta)
    - @task
    - TaskSetMeta
    - TaskSet
    - SequentialTaskSet
    - DefaultTaskSet
  - users.py
    - LocustMeta
    - Locust
    - HttpLocust

At first I thought it would be strange to have the users module (plural) inside a user package (singular), but after giving it some more thought I think it makes sense, since users.py would contain different User implementations, while the user package would be a namespace for all sort of stuff related to user simulation.

[cyberw]

Hm, why is that the case?

I mean like this:

instead of like this:

Of course, there is nothing stopping our users from importing wait_time instead of locust, just because they are exposed in both places, but "There should be one—and preferably only one—obvious way to do it" :)

[cyberw]

Also, can we nuke the old wait api? and its tests in test_old_wait_api.py...

[heyman]

I mean like this:

Ah.

"There should be one—and preferably only one—obvious way to do it" :)

I think having import shortcuts for a few very common functions/classes is an acceptable exception :). There's also the "Although practicality beats purity." 😉

Also, Flask does the same . Doesn't mean it's right, but at least it's such a hugely popular project, so many people will at least recognize it from there.

Also, can we nuke the old wait api? and its tests in test_old_wait_api.py...

Yes 👍

[cyberw]

"There should be one—and preferably only one—obvious way to do it" :)

"Although practicality beats purity." 😉

"The good thing about standards is that there are so many to choose from" ;)

Ok, but flask recommends users to import from the top level.

Maybe we should too? I dislike the current way of providing a "shortcut" but documenting/recommending the "complex" version :)

[cyberw]

Hmm. Github didnt link this in #1329

[cyberw]

Hmm. Moving stuff should probably wait for #1314 to be merged, and because it doesnt change anything in the public API it could be done at a later time. So I'll remove the 1.0 milestone from this.

[heyman]

"The good thing about standards is that there are so many to choose from" ;)

haha :)

Ok, but flask recommends users to import from the top level.
Maybe we should too?

Sounds good! I think we actually do import them from the top level in most of the examples in the docs, but we should make sure that we're consistent.