Summary
The whole instance surface of EnvClient is dual-mode (works from both sync and async callers) thanks to _dispatch / _AutoAsyncResult / _run_sync — reset, step, state, close, connect all resolve correctly whether or not there's a running event loop.
But the constructors that bootstrap a container are async-only:
EnvClient.from_docker_image() and EnvClient.from_env() are plain async def classmethods — no dual-mode wrapper. Calling them from synchronous code returns an un-awaited coroutine.- There is no public getter for the container URL (only the private
self._base_url, set via self._set_base_url(); the previously-public EnvClient.base_url was removed in the core refactor).
The net effect: a synchronous consumer can use the entire client except the one thing that's advertised as the ergonomic one-liner (Env.from_docker_image("...")). There is no from_docker_image_sync, the async classmethod can't be awaited from a sync loop, and even if you could construct the client you can't read its URL back through a public API.
Why this matters (concrete consumer: TRL)
The TRL GRPO examples run inside a synchronous rollout loop, so they can't await from_docker_image(...). To bootstrap a docker-image env they now have to drop down to the provider and hand-roll the exact sequence that from_docker_image already does internally:
# TRL examples/scripts/openenv/{catch,sudoku}.py
provider = LocalDockerProvider()
env_url = provider.start_container(args.env_image)
provider.wait_for_ready(env_url)
# ... then build a sync client with base_url=env_url and use it via _dispatchCompare with EnvClient.from_docker_image internals:
provider = LocalDockerProvider()
base_url = provider.start_container(image, **kwargs)
provider.wait_for_ready(base_url)
client = cls(base_url=base_url, provider=provider)
await client.connect()
The first three lines are identical. So sync consumers end up duplicating OpenEnv's own bootstrap logic. This is not just verbose — it's a maintenance hazard: if the bootstrap sequence gains a step (auth, retries, a different readiness check, a changed default provider), the hand-rolled copies silently diverge and won't inherit it.
(For reference, OpenEnv's own examples/browsergym_harness_eval_common.py uses the provider directly too, which confirms this is the current de-facto sync path — but it's a lower layer than the advertised facade.)
Current workaround: huggingface/trl#6329 — the TRL examples bypass from_docker_image and bootstrap via LocalDockerProvider directly, as shown above.
Proposed fixes (any one closes the gap)
- Make the constructors dual-mode like the rest of the surface (preferred, most consistent):
from_docker_image / from_env return an awaitable in async code and a fully-connected client in sync code.
- Add sync variants, e.g.
from_docker_image_sync() / from_env_sync().
- Re-expose a public
base_url (read-only property over _base_url), so a sync caller that bootstraps via the provider can still round-trip through the client instead of tracking the URL separately.
(1) is the cleanest — it restores symmetry: today every instance method is dual-mode, only the constructors are not.
Context
Summary
The whole instance surface of
EnvClientis dual-mode (works from both sync and async callers) thanks to_dispatch/_AutoAsyncResult/_run_sync—reset,step,state,close,connectall resolve correctly whether or not there's a running event loop.But the constructors that bootstrap a container are async-only:
EnvClient.from_docker_image()andEnvClient.from_env()are plainasync defclassmethods — no dual-mode wrapper. Calling them from synchronous code returns an un-awaited coroutine.self._base_url, set viaself._set_base_url(); the previously-publicEnvClient.base_urlwas removed in thecorerefactor).The net effect: a synchronous consumer can use the entire client except the one thing that's advertised as the ergonomic one-liner (
Env.from_docker_image("...")). There is nofrom_docker_image_sync, the async classmethod can't be awaited from a sync loop, and even if you could construct the client you can't read its URL back through a public API.Why this matters (concrete consumer: TRL)
The TRL GRPO examples run inside a synchronous rollout loop, so they can't
await from_docker_image(...). To bootstrap adocker-imageenv they now have to drop down to the provider and hand-roll the exact sequence thatfrom_docker_imagealready does internally:Compare with
EnvClient.from_docker_imageinternals:The first three lines are identical. So sync consumers end up duplicating OpenEnv's own bootstrap logic. This is not just verbose — it's a maintenance hazard: if the bootstrap sequence gains a step (auth, retries, a different readiness check, a changed default provider), the hand-rolled copies silently diverge and won't inherit it.
(For reference, OpenEnv's own
examples/browsergym_harness_eval_common.pyuses the provider directly too, which confirms this is the current de-facto sync path — but it's a lower layer than the advertised facade.)Current workaround: huggingface/trl#6329 — the TRL examples bypass
from_docker_imageand bootstrap viaLocalDockerProviderdirectly, as shown above.Proposed fixes (any one closes the gap)
from_docker_image/from_envreturn an awaitable in async code and a fully-connected client in sync code.from_docker_image_sync()/from_env_sync().base_url(read-only property over_base_url), so a sync caller that bootstraps via the provider can still round-trip through the client instead of tracking the URL separately.(1) is the cleanest — it restores symmetry: today every instance method is dual-mode, only the constructors are not.
Context
from_docker_imageand the removedbase_url).