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

Add base url path param #2925

Closed
wants to merge 7 commits into from

Conversation

Grommash9
Copy link

Closes #2909

@Grommash9
Copy link
Author

Grommash9 commented Oct 6, 2024

WEB UI is working fine, I can run it, connect to it, I can have worker and tasks is running, I can see all the stats

But I can use any param for base url for worker now, it will be working anyway and it's confusing to me, because I am expecting it to work only if the both are same

Maybe somebody can double check it after me? I am not sure I am doing it in the right way

version: '3'

services:
  master:
    build:
      context: /home/oleksandr/vscode_projects/locust/
      dockerfile: Dockerfile
    ports:
     - "8089:8089"
    volumes:
      - ./:/mnt/locust
    command: -f /mnt/locust/locustfile.py --master -H http://master:8089 --base-url /locust
  
  worker:
    build:
      context: /home/oleksandr/vscode_projects/locust/
      dockerfile: Dockerfile
    volumes:
      - ./:/mnt/locust
    command: -f /mnt/locust/locustfile.py --worker --master-host master --base-url /locust

I am using docker file like that for my local tests

So I am confused what it's also working like that:

image

But I am expecting to get an error because I am not able to connect to master, or am I missing something?

@Grommash9
Copy link
Author

I we able to do it with tcp and ZMQ?

I know we should do some changes here, but how?

image

@cyberw
Copy link
Collaborator

cyberw commented Oct 6, 2024

"Base url" is not a good name, because that is what is used for HttpUser / -H parameter. Maybe ui_base_url?

Needs tests.

@cyberw
Copy link
Collaborator

cyberw commented Oct 6, 2024

I we able to do it with tcp and ZMQ?

I know we should do some changes here, but how?

Maybe I'm confused, but why would this relate to zmq at all? Are you trying to host multiple locust master instances on the same machine?

@Grommash9
Copy link
Author

It's not only about UI, service will be running on that path, so workers should use it to be able to connect with master

I am not sure so far are we able to do that or not

@andrewbaldwin44
Copy link
Collaborator

I agree with @cyberw, we should use a new variable name such as ui_base_url. Renaming arguments could cause a lot of confusion and possibly break for some users when upgrading their locust version.

The workers should not need to be concerned with this base url as only master exposes the web UI

@Grommash9 Grommash9 closed this Oct 8, 2024
@@ -491,20 +497,21 @@ def ensure_user_class_name(config):
else:
web_host = options.web_host
if web_host:
logger.info(f"Starting web interface at {protocol}://{web_host}:{options.web_port}")
logger.info(f"Starting web interface at {protocol}://{web_host}:{options.web_port}{options.base_url}")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think url doesn't actually make sense in this case, URL refers to the entire composition (e.g. protocol://domain:port/path?query=params), in this case base_path would be more accurate

@@ -165,7 +166,7 @@ def send_assets(path):

return send_from_directory(directory, path)

@app.route("/")
@app.route(f"{self.base_url}/")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If self.base_url is / (as it is by default) we will end up with @app.route("//") right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hard coded path make it impossible to host the UI on a path (instead of the domain root)
3 participants