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

Runner crash on first request on Linux (and maybe others) #42

Closed
WinPlay02 opened this issue Jan 25, 2024 · 6 comments · Fixed by #43
Closed

Runner crash on first request on Linux (and maybe others) #42

WinPlay02 opened this issue Jan 25, 2024 · 6 comments · Fixed by #43
Assignees
Labels
bug 🪲 Something isn't working released Included in a release

Comments

@WinPlay02
Copy link
Contributor

WinPlay02 commented Jan 25, 2024

Describe the bug

The runner crashes on first request (usually program message)

To Reproduce

  1. Open VSCode with Safe-DS extension
  2. Click on Run button for any Safe-DS source file

Expected behavior

The runner executes the program

Screenshots (optional)

No response

Additional Context (optional)

I would propose to replace the (legacy) flask stack with a quart (successor in a way of flask) stack that does not require monkey patching python internals. This has the added benefit to allow async python functions and in turn being possibly more efficient.

Furthermore, a migration to the socket.io protocol is still possible although not as easy as changing the dependency in flask.

@WinPlay02 WinPlay02 added the bug 🪲 Something isn't working label Jan 25, 2024
@WinPlay02 WinPlay02 self-assigned this Jan 25, 2024
@lars-reimann
Copy link
Member

Is this related to the issue reported by @SmiteDeluxe?

@lars-reimann
Copy link
Member

lars-reimann commented Jan 25, 2024

Websockets seem to be supported out-of-the-box by quart. How could socket.io be integrated (just spto ensure we don't lose options by migrating)? I guess python-socketio could be used for this purpose?

@lars-reimann
Copy link
Member

litestar might also be a lightweight ASGI framework to consider.

@WinPlay02
Copy link
Contributor Author

Is this related to the issue reported by @SmiteDeluxe?

This is the same issue

Websockets seem to be supported out-of-the-box by quart. How could socket.io be integrated (just see to ensure we don't lose options by migrating? I guess python-socketio could be used for this purpose?

The python-socketio library is the way to go, exactly. python-socketio received support for ASGI a long time ago.

litestar might also be a lightweight ASGI framework to consider.

I chose quart as it is the successor to flask and it seemed the most lightweight. I additionally also considered fastapi.

@WinPlay02
Copy link
Contributor Author

Also see the fix-linux-startup-error branch, for a preview.

Furthermore, a migration to the socket.io protocol is still possible although not as easy as changing the dependency in flask.

The potential future usage of was socket.io was considered in the choice of quart.

lars-reimann pushed a commit that referenced this issue Jan 26, 2024
Closes #42 

### Summary of Changes

- replaces flask with quart
- rewrite some tests
- added test against crashing
- solution should be more scalable as it replaces wsgi with asgi

---------

Co-authored-by: megalinter-bot <129584137+megalinter-bot@users.noreply.github.com>
lars-reimann pushed a commit that referenced this issue Jan 26, 2024
## [0.5.0](v0.4.0...v0.5.0) (2024-01-26)

### Features

* added json serializer that encodes tables and images ([#29](#29)) ([054cca4](054cca4)), closes [#20](#20)
* Memoization ([#38](#38)) ([2a26b48](2a26b48))
* Replace flask with quart ([#43](#43)) ([5520b68](5520b68)), closes [#42](#42)
* support placeholder queries that only request a subset of data ([#39](#39)) ([dae57dc](dae57dc))
* update to safe-ds 0.17.1 + server refactor ([#37](#37)) ([1bcad07](1bcad07))

### Bug Fixes

* allow multiple connections to work with the runner ([#31](#31)) ([64685a3](64685a3))
@lars-reimann
Copy link
Member

🎉 This issue has been resolved in version 0.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@lars-reimann lars-reimann added the released Included in a release label Jan 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🪲 Something isn't working released Included in a release
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants