Integrate streaming as a Skyvern Service only in LINUX or WSL platforms#4681
Integrate streaming as a Skyvern Service only in LINUX or WSL platforms#4681joseluisll wants to merge 22 commits intoSkyvern-AI:mainfrom
Conversation
|
Screenshot of the applied fixed. The streaming now works!.
@wintonzheng can you please review this minimal fix?. Thank you. |
…ndecies to the Dockerfile of the backend
…-initialize-forgeapp-streaming
…m/joseluisll/skyvern into fix-initialize-forgeapp-streaming
|
I have solved also why the live streaming was not working:
All in all, I check that the tests pass, and also that both the screencapture and the live streaming works using the improved docker image. |
AronPerez
left a comment
There was a problem hiding this comment.
Looks really good, like this a lot. Have some questions
No need for using the shell, call the script directly
|
Up to now, I have integrated the streaming service inside the forge_app, and used the start_event to start the service loop. It works, but with the limitations of the screencapture for 1 xfvb display 99. This is the same level of functionality, but now the run_streaming.py is obsolete and not used at all. |
|
we should have a fix for the forge app start call in streaming script: #4703 it looks like this PR is also introducing another functionality here to auto stream screenshot. can we make an update for the title and description of this pr? |
|
@joseluisll nice work |
The snapshot functionality was already there in the run_streaming.py, the screencapture for collecting images to the workflow steps. I just integrated it into the StreamingService. |
It is obsolete, not used any more. The functionality is incorporated into the Streaming service.
Capture streaming_service in a local variable before defining closures to satisfy mypy's type narrowing for StreamingService | None. Co-authored-by: openhands <openhands@all-hands.dev>
|
@suchintan Please review the PR if possible, to approve? |
| True if capture succeeded, False otherwise. | ||
| """ | ||
| try: | ||
| subprocess.run( |
There was a problem hiding this comment.
subprocess.run is synchronous. We'd block the entire event loop for the duration of each screenshot capture. It's basically running every second.
Maybe wrap this in asyncio.create_subprocess_shell() or asyncio.to_thread()
entrypoint-skyvern.sh
Outdated
| @@ -55,8 +55,10 @@ export DISPLAY=:99 | |||
| Xvfb :99 -screen 0 1920x1080x16 & | |||
There was a problem hiding this comment.
Xvfb is started twice with different color depths. scripts/start_vnc_streaming.sh starts in 24-bit color, and we start in 16-bit here. Any reason we need both, or can't just rely on ./scripts/start_vnc_streaming.sh directly?
There was a problem hiding this comment.
In run_streaming.py we have an await initialize_skyvern_state_file(task_id=None, workflow_run_id=None, organization_id=None) check to ensure the state file exists at startup. Do we need that here too?
There was a problem hiding this comment.
The point of this PR is incorporate run_streaming.py as a STREAMING_SERVICE. The run_streaming.py was an external script that created an app object. I make this proposal to integrate it as an internal service, following the same pattern of the rest of services, so it is integrated in the forge_app like the rest of services.
This PR makes the run_streaming.py obsolete.
| AGENT_FUNCTION: AgentFunction | ||
| PERSISTENT_SESSIONS_MANAGER: PersistentSessionsManager | ||
| BROWSER_SESSION_RECORDING_SERVICE: BrowserSessionRecordingService | ||
| STREAMING_SERVICE: StreamingService | None |

Goal
The goal of this PR is to activate the Streaming functionality without changing the behavior or the structure of the original code.
This means that he following known limitations are:
Observations
This PR solves issue #4296. The streaming service could not startup due to broken app context and missing packages into the docker image. Also, the entrypoint did not start the streaming servicies.
Here is the trace that appeared in the log:
Changes
Future work
I would like to make future changes to improve the integration of the StreamingService into the App.