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

make sure the current working dir is in the sys.path #484

Merged
merged 2 commits into from Oct 31, 2019

Conversation

@pwnage101
Copy link
Contributor

pwnage101 commented Sep 27, 2016

rationale

I maintain multiple load tests, each with their own locustfile. They each
share helper utilities located at the project root, but locust/main.py loads
the locustfile in a way that does not make importing the helpers easy. I have
to add the following line to every locustfile to make sure the helper utilities
are importable:

sys.path.append(os.getcwd())

Then I invoke locust from the project root:

locust -f path/to/locustfile

I think it's understandable why locust/main.py inserts the locustfile
directory into the sys.path before loading it. I also think it would be
harmless to also make sure the current working directory is in the sys.path.

minimal example

directory structure:

$ find
.
./__init__.py
./helpers
./helpers/__init__.py
./loadtests
./loadtests/__init__.py
./loadtests/test1
./loadtests/test1/locustfile.py
./loadtests/test1/__init__.py
./loadtests/test2
./loadtests/test2/locustfile.py
./loadtests/test2/__init__.py

helpers/__init__.py:

print("helpers imported!")

loadtests/test1/__init__.py:

from locustfile import MyLocustClass

loadtests/test1/locustfile.py:

"""
Load test suite 1
"""
from locust import Locust, task, TaskSet

# our custom shared helper modules
import helpers


class MyTaskSet(TaskSet):
    @task(1)
    def some_task(self):
        pass

class MyLocustClass(Locust):
    task_set = MyTaskSet

testing running locust without this patch

Before this change, the common helpers module could not be found.

$ locust -f loadtests/test1
[2016-09-27 14:00:48,712] carbon/ERROR/stderr: Traceback (most recent call last):
[2016-09-27 14:00:48,712] carbon/ERROR/stderr: File "/home/tsankey/Documents/perf-357/locust_minimal_example/venv/bin/locust", line 9, in <module>
[2016-09-27 14:00:48,712] carbon/ERROR/stderr:
[2016-09-27 14:00:48,712] carbon/ERROR/stderr: load_entry_point('locustio==0.7.3', 'console_scripts', 'locust')()
[2016-09-27 14:00:48,712] carbon/ERROR/stderr: File "/home/tsankey/Documents/perf-357/locust_minimal_example/venv/local/lib/python2.7/site-packages/locustio-0.7.3-py2.7.egg/locust/main.py", line 349, in main
[2016-09-27 14:00:48,712] carbon/ERROR/stderr:
[2016-09-27 14:00:48,712] carbon/ERROR/stderr: docstring, locusts = load_locustfile(locustfile)
[2016-09-27 14:00:48,712] carbon/ERROR/stderr: File "/home/tsankey/Documents/perf-357/locust_minimal_example/venv/local/lib/python2.7/site-packages/locustio-0.7.3-py2.7.egg/locust/main.py", line 321, in load_locustfile
[2016-09-27 14:00:48,713] carbon/ERROR/stderr:
[2016-09-27 14:00:48,713] carbon/ERROR/stderr: imported = __import__(os.path.splitext(locustfile)[0])
[2016-09-27 14:00:48,713] carbon/ERROR/stderr: File "/home/tsankey/Documents/perf-357/locust_minimal_example/venv/local/lib/python2.7/site-packages/gevent-1.1.1-py2.7-linux-x86_64.egg/gevent/builtins.py", line 93, in __import__
[2016-09-27 14:00:48,713] carbon/ERROR/stderr:
[2016-09-27 14:00:48,713] carbon/ERROR/stderr: result = _import(*args, **kwargs)
[2016-09-27 14:00:48,713] carbon/ERROR/stderr: File "/home/tsankey/Documents/perf-357/locust_minimal_example/example/loadtests/test1/__init__.py", line 1, in <module>
[2016-09-27 14:00:48,713] carbon/ERROR/stderr:
[2016-09-27 14:00:48,713] carbon/ERROR/stderr: from locustfile import MyLocustClass
[2016-09-27 14:00:48,713] carbon/ERROR/stderr: File "/home/tsankey/Documents/perf-357/locust_minimal_example/venv/local/lib/python2.7/site-packages/gevent-1.1.1-py2.7-linux-x86_64.egg/gevent/builtins.py", line 93, in __import__
[2016-09-27 14:00:48,713] carbon/ERROR/stderr:
[2016-09-27 14:00:48,713] carbon/ERROR/stderr: result = _import(*args, **kwargs)
[2016-09-27 14:00:48,713] carbon/ERROR/stderr: File "/home/tsankey/Documents/perf-357/locust_minimal_example/example/loadtests/test1/locustfile.py", line 10, in <module>
[2016-09-27 14:00:48,713] carbon/ERROR/stderr:
[2016-09-27 14:00:48,713] carbon/ERROR/stderr: import helpers
[2016-09-27 14:00:48,713] carbon/ERROR/stderr: File "/home/tsankey/Documents/perf-357/locust_minimal_example/venv/local/lib/python2.7/site-packages/gevent-1.1.1-py2.7-linux-x86_64.egg/gevent/builtins.py", line 93, in __import__
[2016-09-27 14:00:48,713] carbon/ERROR/stderr:
[2016-09-27 14:00:48,713] carbon/ERROR/stderr: result = _import(*args, **kwargs)
[2016-09-27 14:00:48,713] carbon/ERROR/stderr: ImportError
[2016-09-27 14:00:48,713] carbon/ERROR/stderr: :
[2016-09-27 14:00:48,713] carbon/ERROR/stderr: No module named helpers
[2016-09-27 14:00:48,713] carbon/ERROR/stderr:

testing running locust with this patch

$ locust -f loadtests/test1
[2016-09-27 13:59:24,798] carbon/INFO/stdout: helpers imported!
[2016-09-27 13:59:24,798] carbon/INFO/stdout:
[2016-09-27 13:59:24,798] carbon/INFO/locust.main: Starting web monitor at *:8089
[2016-09-27 13:59:24,799] carbon/INFO/locust.main: Starting Locust 0.7.3
@pwnage101

This comment has been minimized.

Copy link
Contributor Author

pwnage101 commented Sep 28, 2016

@justiniso

This comment has been minimized.

Copy link
Member

justiniso commented Nov 8, 2016

Your use case is very similar to how I and many others use locust. Unfortunately, I'd be worried about this breaking people's integrations or having unintended side effects depending on where you are running from and if you have modules or packages with the same name across your project.

What I'd like to see before this gets merged is some best practices documentation for structuring a non-flat locust project. Trying to standardize project structures and import paths to some extent will make them easier to support in the future. I made #500 to track that separately, but feel free to take a first stab if you're up for it.

@mbeacom

This comment has been minimized.

Copy link
Member

mbeacom commented Dec 14, 2016

First off, thanks!
Just a bump!
Will this be revisited now that #500 is fulfilled via #501 ?

thaffenden pushed a commit to thaffenden/locust that referenced this pull request Feb 9, 2017
@justiniso justiniso self-assigned this Feb 13, 2017
@justiniso

This comment has been minimized.

Copy link
Member

justiniso commented Feb 13, 2017

@cgoldberg what do you think about this one?

thaffenden pushed a commit to thaffenden/locust that referenced this pull request Feb 16, 2017
thaffenden pushed a commit to thaffenden/locust that referenced this pull request Feb 20, 2017
@justiniso justiniso removed their assignment May 14, 2018
@thaffenden

This comment has been minimized.

Copy link
Contributor

thaffenden commented Jul 4, 2018

Sorry to bump on this one, but is it likely to get merged at all? @justiniso @cgoldberg
(I know it can be done manually in test files, but it'd be nice not to have to).

@cyberw

This comment has been minimized.

Copy link
Contributor

cyberw commented Oct 20, 2019

@heyman What is your opinion on this? I agree that it may be a bit disruptive to existing tests because it changes how locust runs. But I do think people expect to be able to import modules in the same fashion as if they had run python somelocustfile.py instead of locust -f somelocustfile.py, so in all I'd say I'm +1

@heyman

This comment has been minimized.

Copy link
Member

heyman commented Oct 20, 2019

I understand the issue but I'm a little bit hesitant because of potential negative effects it might have (that are currently unknown to me). But I think I'm +0 at the moment.

#1112 that was just merged makes it possible to avoid the issue by starting locust using python -m locust -f locustfiles/myfile.py ... instead where imports from CWD should work.

@cyberw

This comment has been minimized.

Copy link
Contributor

cyberw commented Oct 31, 2019

@heyman merge or reject, I think it is up to you :)

Like I said, I'm +1, but I dont need this change myself, so it's not that important to me...

@heyman

This comment has been minimized.

Copy link
Member

heyman commented Oct 31, 2019

Okay, let's merge it. It solves a real issue, and if we realize that it creates other issues at some point in the future we can always re-evaluate :).

@heyman heyman merged commit bcaff5c into locustio:master Oct 31, 2019
1 check was pending
1 check was pending
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.