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

fix --html report in web mode #1992

Merged
merged 1 commit into from
Jan 30, 2022
Merged

fix --html report in web mode #1992

merged 1 commit into from
Jan 30, 2022

Conversation

uddmorningsun
Copy link
Contributor

@uddmorningsun uddmorningsun commented Jan 29, 2022

If locust is started by --headful argument(web mode), main_greenlet.join() will serve forever. get_html_report will be inaccessable.

If running locust will be CTRL+C, get_html_report will not be received by catching the exception.

In order to keep simple, get_html_report is dealt with shutdown() according to --html argument value.

Signed-off-by: Chenyang Yan memory.yancy@gmail.com

Edit: Fixes #1944

@cyberw
Copy link
Collaborator

cyberw commented Jan 29, 2022

command line help text is a bit verbose, "Store HTML report to file path specified" should be enough, does that sound ok?

If you havent already, please test this in "regular" mode (run a test from the gui, stop it from the gui) as well as using --autostart

@uddmorningsun
Copy link
Contributor Author

"Store HTML report to file path specified"

It's OK

I've forgot stop button in web ui:sweat_smile:. But there's a problem in locust.web.py:WebUI.stop, only runner is stopped, but there has always been found greenlet process. If sending /stop, you will see process hang up. Maybe add self.greenlet.kill(block=True)?

And, what do you think about dealing with ctrl-c for get_html_report

@cyberw
Copy link
Collaborator

cyberw commented Jan 29, 2022

Good question. Unfortunately I'm not completely up to speed on those things, and I dont have time to dig into it, so your guess is as good as mine :) (I will of course be happy to review, once it is working)

but there has always been found greenlet process

I dont understand that sentence...

If running locust will be CTRL+C, `get_html_report` will be received by catching the exception.

Signed-off-by: Chenyang Yan <memory.yancy@gmail.com>
@uddmorningsun
Copy link
Contributor Author

Sorry, my broken English😅 It means there's a bug with /stop in web.py. Process will hang up in terminal window while clicking stop button in web ui.

I updated, could you help me review it?

@@ -174,6 +174,8 @@ def stop():
self._swarm_greenlet.kill(block=True)
self._swarm_greenlet = None
environment.runner.stop()
if self.greenlet is not None:
self.greenlet.kill(block=True)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm... I'm worried this might cause issues with repeated gui runs. Have you tried that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What does meaning of repeated gui runs? I guess that different --web-port to run?

I have tried that it's normal to start again after sending the stop button, even for different ports.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I have tried that it's normal to start again after sending the stop button, even for different ports.

That's all I meant :) Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, it's my fault 😐 for the issue #1995. Again, I understood the meaning of with repeated gui runs, run multiple tests in one running locust environment.

@cyberw cyberw changed the title Fix issue 1944 -- make config option of html effect fix --html report in web mode (#1944) Jan 30, 2022
@cyberw cyberw changed the title fix --html report in web mode (#1944) fix --html report in web mode Jan 30, 2022
@cyberw cyberw merged commit a3e8af7 into locustio:master Jan 30, 2022
@cyberw
Copy link
Collaborator

cyberw commented Feb 3, 2022

Yea, that didnt work very well, causing the issue #1995 I'm reverting the greenlet killing part.

cyberw added a commit that referenced this pull request Feb 3, 2022
@chy8803
Copy link

chy8803 commented Feb 3, 2022

Yea, that didnt work very well, causing the issue #1995 I'm reverting the greenlet killing part.

This PR caused the master pod restarting and resetting the data. After clicking Stop button UI, /stats/report becomes unaccessible since the process is stopped.

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.

--html doesnt work in web mode
3 participants