Add support for executing commands inside running Docker containers #31
Conversation
githoober
changed the title
githoober
changed the title
|
@ckreiling, please, please check at your earliest convenience. Thanks. It is working on my side. |
| - `remove_container` | ||
| - `exec_in_container` | ||
|
|
||
| #### New: `exec_in_container` |
There was a problem hiding this comment.
We don't have this "New" section for other tools - please remove
|
|
||
| if args.detach: | ||
| result = {"status": "detached", "exec_id": exec_result.id} | ||
| elif args.stream: |
There was a problem hiding this comment.
What is the benefit to supporting stream if we wait to collect all the output and return a response to the LLM? This seems like an option that shouldn't be relevant to this implementation
| exec_params = { | ||
| "cmd": args.command, | ||
| "stdout": True, | ||
| "stderr": True, | ||
| "stdin": args.stdin, | ||
| "tty": args.tty, | ||
| "privileged": args.privileged, | ||
| "user": args.user, | ||
| "environment": args.environment, | ||
| "workdir": args.working_dir, | ||
| "detach": args.detach, | ||
| "stream": args.stream, | ||
| "socket": args.socket, | ||
| "demux": args.demux, | ||
| } |
There was a problem hiding this comment.
This should just be a call to args.model_dump(). What I've done historically is make the Pydantic model (in this case ExecInContainerInput) map directly to the input kwargs on the corresponding Docker client, and use the same defaults as the client. it ends up being cleaner & clearer to the implementor AND the model what the default behavior is, at the cost of a little redundancy
| "stdout": True, | ||
| "stderr": True, |
There was a problem hiding this comment.
The LLM should be able to set these options; perhaps it doesn't care about the output and can save some input tokens by opting to skip stdout and stderr
|
Hello @githoober thank you for your contributions! I've left a few comments above that should be addressed before I am comfortable merging these changes. Mainly I am looking to align your additions with existing best-practices I've established |
|
@githoober let me know if you want any further guidance to get this merged! |
Fixes #22