Add a quit command

[Enderdead]

I'm using BOPTEST for my personal research and would like to structure my experiments with only ONE script to execute. The simulator architecture requires us to launch the server on one side and the client on the other side. These two processes can be merged into one script thanks to the subprocess library. However, as the Flask server is designed to function as a persistent server, it won't terminate itself if the client ends its tasks on this instance. This means that my merged script cannot finish and requires a keyboard interrupt.

To solve this issue, I propose the addition of a quit command to shut down the server remotely. Adding this command wouldn't alter the user experience but it would offer a new way to manage the server. I have already implemented the REST command, but before making a pull request, I would appreciate your feedback on this.

Thank you in advance for your attention.

[dhblum]

Thank you for reporting and suggesting. I understand the need. In boptest-service we have the endpoint /stop. See the API readme there. I'd propose using the same API call here to shutdown the running test container, which is comparable to the same request in Service. @kbenne @JavierArroyoBastida Thoughts?

[javiarrobas]

@Enderdead what would be the difference between the existing /stop endpoint and the command you propose? it'd be maybe good idea to send a draft pull request to better understand.

[dhblum]

@JavierArroyoBastida The /stop endpoint only exists when using Service right now. This would add it to the core API when a test case is deployed locally.

[javiarrobas]

I see. Thanks for clarifying! is there any backward incompatibility this could bring? otherwise I think it's an added value to have it.

[Enderdead]

From my standpoint, there should be no compatibility issues, as the new feature introduces an independent command. I have already incorporated this command into my fork of BOPTEST (see here: 2070737).
I invite you to review my implementation if needed. I will await your approval before submitting a merge request.

[dhblum]

@Enderdead I thought about this a little more and wondered how you start a test case with your script, even after quitting one? Test cases are started with docker-compose up and can be similarly shut down with docker-compose down, which is actually preferred over the keyboard interrupt because it removes the network as well. I ask because, I wonder if your script uses docker-compose up to start a test, it could similarly use docker-compose down to stop it?

[Enderdead]

I'm using the docker command directly instead of docker-compose to update the port, so yes, your approach works. The docker stop command is able to stop the server, but it takes quite long, about 10 seconds. The docker kill command works as intended but it's not really clean.

I will proceed with the docker kill command.

I'm not sure if you still want to pursue the idea of a quit command, but as it stands, it's fine.

I close the issue for now.

[dhblum]

Ok thanks a lot @Enderdead. The initial value I thought the quit command would have is for the most part nullified I think due to the need for a docker command to start up a test case anyway. I suppose there's an argument for separating the starting of the docker container and then starting up of a specific test case, but that's a more intrusive change which I don't think is a priority right now. I'm inclined not to pursue this, nor the quit command, at the moment.