Add wsgi.py analogous to asgi.py#145
Conversation
There was a problem hiding this comment.
Considering that we could add more features to these gateway servers, I think it is better to put it in the subdirectory following the regular packages structure (same for asgi.py). We can still have an asgi.py in the top-level directory which just imports from this directory for backward compatibility.
| return True | ||
|
|
||
| def _fill(self) -> None: | ||
| from pyodide.ffi import run_sync |
There was a problem hiding this comment.
Should we guard against Pyodide 0.26.0a2 which doesn't support run_sync?
There was a problem hiding this comment.
And I assume you mean 0.26?
There was a problem hiding this comment.
And I assume you mean 0.26?
oh yeah right.
| def close(self) -> None: | ||
| if not self.closed: | ||
| try: | ||
| self._reader.releaseLock() |
There was a problem hiding this comment.
I noticed that workerd raises error when there is a pending read. It is in the ReadableStream spec, but it seems the behavior is different in Node.js / workerd (so I believe AI agents are not very good at it).
so probably this function should make sure there is no pending read by calling run_sync(await self._reader.cancel() first.
Also, we need to make sure the lock is released when close is not called.
| def _can_stream() -> bool: | ||
| """Return True if we can lazily stream the body via run_sync""" | ||
| try: | ||
| from pyodide.ffi import can_run_sync |
There was a problem hiding this comment.
This should be always true in workerd with Pyodide >= 0.28.2, otherwise it will not exist at all.
|
|
||
| def build_environ( | ||
| req: "Request | js.Request", | ||
| env: Any, |
There was a problem hiding this comment.
Maybe give dict[str, Any] type?
| "SCRIPT_NAME": "", | ||
| "PATH_INFO": path_info, | ||
| "QUERY_STRING": query_string, | ||
| "SERVER_NAME": url.hostname or "localhost", |
There was a problem hiding this comment.
Will this ever be empty?
| "wsgi.run_once": False, | ||
| # Cloudflare-specific extension so handlers can reach bindings, mirroring | ||
| # the `scope["env"]` convention used by asgi.py. | ||
| "workers.env": env, |
There was a problem hiding this comment.
(No action required for this PR) We should properly document this later and never change.
| raise AssertionError("start_response() called more than once") | ||
|
|
||
| response_state["status"] = status | ||
| response_state["headers"] = list(response_headers) |
There was a problem hiding this comment.
why do we need to convert to list?
|
|
||
| result = app(environ, start_response) | ||
| try: | ||
| body_chunks = list(write_buffer) |
| raise RuntimeError("The WSGI application did not call start_response()") | ||
|
|
||
| return _make_js_response( | ||
| response_state["status"], response_state["headers"], b"".join(body_chunks) |
There was a problem hiding this comment.
The comment says that the write_buffer is only used in the legacy frameworks but it seems we are always using it here.
There was a problem hiding this comment.
Also this would store all the data in the memory. I think we should find a way to properly stream it
|
Thanks for working on this. I think we need a proper testing strategy with enough coverage before merging this, as we release every commit. Otherwise, we can create a temporary branch that we can incrementally improve until we are confident enough to be merged to main. |
2 participants
No description provided.