From abec52abfe45333d127c023b71b65e3309e35c7b Mon Sep 17 00:00:00 2001 From: tobitege Date: Sun, 4 Aug 2024 21:13:37 +0200 Subject: [PATCH] (fix) Revert #3233; more logging in runtimes (#3236) * ServerRuntime: config copy in init * revert #3233 but more logging * get_box_classes: reset order back to previous version * 3 logging commands switched to debug (were info) * runtimes debug output of config on initialization * removed unneeded logger message from _init_container --- opendevin/runtime/client/runtime.py | 5 +---- opendevin/runtime/runtime.py | 1 + opendevin/runtime/server/runtime.py | 1 + tests/unit/test_runtime.py | 25 +++++++++++++++---------- 4 files changed, 18 insertions(+), 14 deletions(-) diff --git a/opendevin/runtime/client/runtime.py b/opendevin/runtime/client/runtime.py index f4782756b9a0..bb5cf0713bfd 100644 --- a/opendevin/runtime/client/runtime.py +++ b/opendevin/runtime/client/runtime.py @@ -1,5 +1,4 @@ import asyncio -import copy import os import tempfile import uuid @@ -50,7 +49,6 @@ def __init__( plugins: list[PluginRequirement] | None = None, container_image: str | None = None, ): - self.config = copy.deepcopy(config) super().__init__( config, event_stream, sid, plugins ) # will initialize the event stream @@ -72,6 +70,7 @@ def __init__( self.container = None self.action_semaphore = asyncio.Semaphore(1) # Ensure one action at a time + logger.debug(f'EventStreamRuntime `{sid}` config:\n{self.config}') async def ainit(self, env_vars: dict[str, str] | None = None): if self.config.sandbox.od_runtime_extra_deps: @@ -151,8 +150,6 @@ async def _init_container( ) volumes = None - logger.info(f'run_as_devin: `{self.config.run_as_devin}`') - if self.config.sandbox.browsergym_eval_env is not None: browsergym_arg = ( f'--browsergym-eval-env {self.config.sandbox.browsergym_eval_env}' diff --git a/opendevin/runtime/runtime.py b/opendevin/runtime/runtime.py index abef41199933..dc6b57c8dd11 100644 --- a/opendevin/runtime/runtime.py +++ b/opendevin/runtime/runtime.py @@ -70,6 +70,7 @@ def __init__( self.config = copy.deepcopy(config) self.DEFAULT_ENV_VARS = _default_env_vars(config.sandbox) atexit.register(self.close_sync) + logger.debug(f'Runtime `{sid}` config:\n{self.config}') async def ainit(self, env_vars: dict[str, str] | None = None) -> None: """ diff --git a/opendevin/runtime/server/runtime.py b/opendevin/runtime/server/runtime.py index 03f77d6245fc..7bdeb616f147 100644 --- a/opendevin/runtime/server/runtime.py +++ b/opendevin/runtime/server/runtime.py @@ -52,6 +52,7 @@ def __init__( self.sandbox = sandbox self._is_external_sandbox = True self.browser: BrowserEnv | None = None + logger.debug(f'ServerRuntime `{sid}` config:\n{self.config}') def create_sandbox(self, sid: str = 'default', box_type: str = 'ssh') -> Sandbox: if box_type == 'local': diff --git a/tests/unit/test_runtime.py b/tests/unit/test_runtime.py index 180a844ecdb8..174c28689cb7 100644 --- a/tests/unit/test_runtime.py +++ b/tests/unit/test_runtime.py @@ -962,13 +962,16 @@ async def _test_ipython_agentskills_fileop_pwd_impl( @pytest.mark.asyncio -async def test_ipython_agentskills_fileop_pwd(temp_dir, box_class, enable_auto_lint): +async def test_ipython_agentskills_fileop_pwd( + temp_dir, box_class, run_as_devin, enable_auto_lint +): """Make sure that cd in bash also update the current working directory in ipython.""" runtime = await _load_runtime( - temp_dir, box_class, enable_auto_lint=enable_auto_lint + temp_dir, box_class, run_as_devin, enable_auto_lint=enable_auto_lint ) await _test_ipython_agentskills_fileop_pwd_impl(runtime, enable_auto_lint) + await runtime.close() await asyncio.sleep(1) @@ -1039,10 +1042,6 @@ async def test_ipython_agentskills_fileop_pwd_with_userdir(temp_dir, box_class): await asyncio.sleep(1) -@pytest.mark.skipif( - TEST_RUNTIME.lower() == 'eventstream', - reason='Skip this if we want to test EventStreamRuntime', -) @pytest.mark.skipif( os.environ.get('TEST_IN_CI', 'false').lower() == 'true', # FIXME: There's some weird issue with the CI environment. @@ -1050,18 +1049,24 @@ async def test_ipython_agentskills_fileop_pwd_with_userdir(temp_dir, box_class): ) @pytest.mark.asyncio async def test_ipython_agentskills_fileop_pwd_agnostic_sandbox( - temp_dir, enable_auto_lint, container_image + temp_dir, box_class, run_as_devin, enable_auto_lint, container_image ): - """Make sure that cd in bash also update the current working directory in ipython.""" + """Make sure that cd in bash also updates the current working directory in iPython.""" + + # NOTE: we only test for ServerRuntime, since EventStreamRuntime + # is image agnostic by design. + if box_class != 'server': + pytest.skip('Skip this if box_class is not server') runtime = await _load_runtime( temp_dir, - # NOTE: we only test for ServerRuntime, since EventStreamRuntime is image agnostic by design. - ServerRuntime, + box_class, + run_as_devin, enable_auto_lint=enable_auto_lint, container_image=container_image, ) await _test_ipython_agentskills_fileop_pwd_impl(runtime, enable_auto_lint) + await runtime.close() await asyncio.sleep(1)