Pass container port instead of host port to Docker#12595
Pass container port instead of host port to Docker#12595enyst merged 2 commits intoOpenHands:mainfrom
Conversation
3d68964 to
1c831aa
Compare
|
Tested with the following prompt and confirmed the agent used port 8011, the App tab displayed the app using the correct externally visible URL (why CORS is mentioned), and also accessible from my browser. The work_host skill prompt could debatably be improved if it included the need for CORS to work correctly with the App tab. "Build a Flask app that displays a pretty 'Hello, World!' webpage, ensuring it runs on an externally visible port and has CORS enabled." 
|
1c831aa to
87494c0
Compare
|
I'v rebased to main to resolve a merge conflict with new host networking mode. The host networking was mismatched and using host_port for one area and container_port for another area. Both are same value so either was OK but I aligned with bridge networking for consistency. |
|
I've confirmed the rebase works correctly for new host networking mode in addition to default bridge networking. WORKER_1/2 still are fixed at 8011/8012 (same as bridge mode) and this is what work_host skill looks like for host networking mode: I don't think the CI failure can be related to my changes but if there is something I can do to address then please let me know. |
WORKER_1/WORKER_2 should be port for apps to listen on so
that they can be accessed externally but currently the
Docker runtime sets this to the unreachable dynamic ports
meant for host side usage.
This also fixes work_host skill in Docker so that it lists
the correct port to listen on so that LLM chooses correct port
when starting servers.
The user has access to the following hosts for accessing
a web application, each of which has a corresponding port:
* http://hostname:X (8011)
* http://hostname:Y (8012)
87494c0 to
295efa5
Compare
|
I re-reviewed and discovered I had mistakenly included uv.lock in commit which was breaking github tests. I also rebased to latest main while fixing that. |
Co-authored-by: Engel Nyst <engel.nyst@gmail.com>
Summary of PR
WORKER_1/WORKER_2 should be port for apps to listen on so that they can be accessed externally but currently the Docker runtime sets this to the unreachable dynamic ports meant for host side usage.
This also fixes work_host skill in Docker so that it lists the correct port to listen on so that LLM chooses correct port when starting servers.
Demo Screenshots/Videos
n/a
Change Type
Checklist
Fixes
Ask OpenHands to create Hello, World flask app with CORS support and run it. It will use WORKER_1 port (dynamic host port) because of work_host skill but app will not be accessible externally and the App tab will not auto-display the app. This fix will cause it to use port 8011 with Docker runtime which will make the app accessible and be visible in App tab.
Resolves #(issue)
n/a
Release Notes