feat: stop ModularServer closes #1642 #1675
Conversation
Implementation ------------------- Python: * ModularServer: add `stop` method * SocketHandler: handle message type "shut_down" (only in debug mode) UI: Add "Shut Down" button * Disable the nav items. * Add Bootstrap alert element. * Send "shut_down" message to WebSocket. Other ------- SocketHandler.on_message: add error handling for message without a 'type' key
|
Here is a 20-second video showing the "Shut Down" button. |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #1675 +/- ##
==========================================
- Coverage 81.69% 81.16% -0.53%
==========================================
Files 18 18
Lines 1393 1407 +14
Branches 271 274 +3
==========================================
+ Hits 1138 1142 +4
- Misses 209 219 +10
Partials 46 46
☔ View full report in Codecov by Sentry. |
|
@reka I have tested your PR and it works! |
| try: | ||
| msg_type = msg["type"] | ||
| except KeyError: | ||
| print("Unexpected message. Key 'type' missing.") |
There was a problem hiding this comment.
I haven't encountered this branch point yet.
I think the safeguard is not necessary. Printing this error message instead of raising the exception results in an information loss about the location in which the error happens.
Replacing with msg_type = msg["type"] should do.
There was a problem hiding this comment.
I haven't encountered this branch point yet.
I ran into this, as I was trying out various changes in mesa/visualization/templates/js/runcontrol.js.
When I sent a message that didn't contain a key "type"
=>
- The SocketHandler threw an error
- The Tornado server shut down.
If we add an error handling, the server will continue running despite of the unexpected message.
I'm not sure whether this is a good or bad thing.
Printing this error message instead of raising the exception results in an information loss about the location in which the error happens.
That's a good point.
Is there more info that should be included in the error message?
Or do you think that raising the exception and shutting down the server is the way to go?
There was a problem hiding this comment.
When I sent a message that didn't contain a key "type"
This is a change that is made by the developer instead of the user, as such, I believe the server should halt violently with tracebacks instead of continue running.
| @@ -246,6 +251,10 @@ def on_message(self, message): | |||
| else: | |||
| self.application.model_kwargs[param] = value | |||
|
|
|||
| elif msg_type == "shut_down": | |||
There was a problem hiding this comment.
Why is it necessary to be in debug mode in order for the shutdown to be functional?
There was a problem hiding this comment.
My way of thoughts was:
- It's generally not a good idea to allow a client to shut down a web server.
- In a local environment, in debug mode, this might be OK.
By the way, I have a general question:
Do I understand correctly that this Tornado server is expected to run only in a local environment, on the user's computer?
Or is it also deployed somewhere as an actual web server? 🤔
There was a problem hiding this comment.
Or is it also deployed somewhere as an actual web server? 🤔
Some people might have done this, that I am not aware of. The relevant issue is #481. We are also currently exploring the possibility of having the UI stack fully in Python (#1622), in a way that can be run on a Jupyter notebook. Hosting Jupyter notebooks is so ubiquitous that there are canned ways to do it securely.
There was a problem hiding this comment.
I am in the opinion that if the pure Python implementation goes well, it might replace the current clunky Tornado server, in a way that is easy to extend, and based on existing established data interaction frameworks. cc: @ankitk50.
Another past attempt at revamping the frontend: #1334 by @Corvince.
* update error message * add a description of the message parameter to the docstring
|
We have shipped Mesa 2.1, and it includes an experimental Solara-based viz frontend that can either run standalone or in a Jupyter notebook. See the usage in https://mesa.readthedocs.io/en/stable/tutorials/visualization_tutorial.html. Your feedback would be appreciated. |
|
It may be viable for pedagogical purpose, but for large grid size, it is a Matplotlib full recreation of the |
|
https://mesa.readthedocs.io/en/stable/tutorials/visualization_tutorial.html doesn't tell how to do it standalone. See https://github.com/projectmesa/mesa-examples/tree/main/examples/boltzmann_wealth_model_experimental#how-to-run instead. |
|
Since we moved vie tornado stuff to its own repo I am going to close this issue But @reka feel free to re-open a PR at https://github.com/projectmesa/mesa-viz-tornado I think this change should still be implemented. The only blocker for me is that it seems like the shut-down button is always visible, even if not in debug-mode. This should be fixed, otherwise it is very bad UX to always see a button you can never use ;) |
Implementation
Python:
stopmethodUI: Add "Shut Down" button
Other
SocketHandler.on_message: add error handling for message without a 'type' key