Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add Runtime for tool calling #1163

Open
wants to merge 49 commits into
base: master
Choose a base branch
from
Open

feat: Add Runtime for tool calling #1163

wants to merge 49 commits into from

Conversation

liuxukun2000
Copy link
Collaborator

@liuxukun2000 liuxukun2000 commented Nov 8, 2024

Description

Add runtime class for tool calling: #1014

  • Docker Runtime
  • Remote HTTP Runtime
  • LLM Guard Runtime

Motivation and Context

Issue: #1014

  • I have raised an issue to propose this change (required for new features and bug fixes)

Types of changes

What types of changes does your code introduce? Put an x in all the boxes that apply:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds core functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (update in the documentation)
  • Example (update in the folder of example)

Checklist

Go over all the following points, and put an x in all the boxes that apply.
If you are unsure about any of these, don't hesitate to ask. We are here to help!

  • I have read the CONTRIBUTION guide. (required)
  • My change requires a change to the documentation.
  • I have updated the tests accordingly. (required for a bug fix or a new feature)
  • I have updated the documentation accordingly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems we need to make the image public

docker.errors.ImageNotFound: 404 Client Error for http+docker://localhost/v1.45/images/create?tag=latest&fromImage=xukunliu%2Fcamel: Not Found ("pull access denied for xukunliu/camel, repository does not exist or may require 'docker login': denied: requested access to the resource is denied")

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Wendong! Just wondering if camel has any plans to build an official image and push it to DockerHub?

camel/runtime/api.py Outdated Show resolved Hide resolved
camel/runtime/api.py Outdated Show resolved Hide resolved
camel/runtime/api.py Outdated Show resolved Hide resolved
camel/runtime/docker_runtime.py Outdated Show resolved Hide resolved
camel/runtime/docker_runtime.py Outdated Show resolved Hide resolved
camel/runtime/docker_runtime.py Outdated Show resolved Hide resolved
camel/runtime/docker_runtime.py Outdated Show resolved Hide resolved
camel/runtime/docker_runtime.py Outdated Show resolved Hide resolved
camel/runtime/docker_runtime.py Show resolved Hide resolved
camel/runtime/llm_guard_runtime.py Outdated Show resolved Hide resolved
camel/runtime/llm_guard_runtime.py Outdated Show resolved Hide resolved
camel/runtime/llm_guard_runtime.py Outdated Show resolved Hide resolved
camel/runtime/llm_guard_runtime.py Outdated Show resolved Hide resolved
camel/runtime/llm_guard_runtime.py Outdated Show resolved Hide resolved
camel/runtime/llm_guard_runtime.py Outdated Show resolved Hide resolved
camel/runtime/utils/ignore_risk_toolkit.py Outdated Show resolved Hide resolved
camel/toolkits/code_execution.py Outdated Show resolved Hide resolved
camel/runtime/remote_http_runtime.py Show resolved Hide resolved
camel/runtime/remote_http_runtime.py Outdated Show resolved Hide resolved
Copy link
Member

@Wendong-Fan Wendong-Fan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @liuxukun2000 ! The core code looks great now, I added one more commit for small docstring and code format polish here: 4c4d4ac , feel free to check. The last thing is the unit test to be added, could you add unit test under test folder? Thanks a lot!

Comment on lines +72 to +86
@app.post(f"/{func.get_function_name()}")
async def dynamic_function(data: Dict, func=func):
redirect_stdout = data.get('redirect_stdout', False)
if redirect_stdout:
sys.stdout = io.StringIO()
response_data = func.func(*data['args'], **data['kwargs'])
if redirect_stdout:
sys.stdout.seek(0)
output = sys.stdout.read()
sys.stdout = sys.__stdout__
return {
"output": json.dumps(response_data),
"stdout": output,
}
return {"output": json.dumps(response_data)}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is RPC a better choice? We can have a simple discussion about it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi xukun, Would it be possible to incorporate the agent part into this example?

@Asher-hss
Copy link
Collaborator

Hi, Xukun,

As a framework, I think Camel shouldn't rely on starting an HTTP service for running tools. If the HTTP port is exposed, it introduces additional security concerns that need to be addressed.

Would it be possible to directly start a Docker container, execute the corresponding tool script (.py), and destroy the container immediately after execution?

@liuxukun2000
Copy link
Collaborator Author

Hi @Asher-hss , thank you for your suggestion and for considering the potential security implications of starting an HTTP service. However, I believe HTTP security concerns are not something the framework itself should manage, as long as the service is used properly and securely. HTTP servers are widely adopted for implementing function calls due to their versatility and ease of integration.
Additionally, using an HTTP server ensures both efficiency and greater flexibility, which is particularly critical in benchmarks where frequent function calls are required. It also allows us to maintain the context of function calls, which would be challenging to achieve with your proposed approach of using ephemeral Docker containers.
Let me know if you'd like to discuss this further or have additional thoughts!

@Asher-hss
Copy link
Collaborator

Asher-hss commented Nov 19, 2024

@liuxukun2000 Hi xukun, I really appreciate the consideration for flexibility—it’s definitely an important factor. That said, I’m also thinking about starting a service inherently aims to create a long-running environment, which might not fully align with the original goals we had in mind for designing this runtime. Then, could you share an example scenario where maintaining the function call context would be particularly useful? Lastly, I’d love to hear your thoughts on what specific use cases you see for adopting an HTTP runtime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

5 participants