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

isolated python code interpreter #619

Merged
merged 5 commits into from
Aug 14, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions docs/genaisrc/genaiscript.d.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion docs/src/components/BuiltinTools.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
<LinkCard title="fs_find_files" description="Finds file matching a glob pattern." href="/genaiscript/reference/scripts/system#systemfs_find_files" />
<LinkCard title="fs_read_file" description="Reads a file as text from the file system." href="/genaiscript/reference/scripts/system#systemfs_read_file" />
<LinkCard title="fs_read_summary" description="Reads a summary of a file from the file system." href="/genaiscript/reference/scripts/system#systemfs_read_summary" />
<LinkCard title="math_eval" description="Evaluates a math expression" href="/genaiscript/reference/scripts/system#systemmath" />

Check failure on line 12 in docs/src/components/BuiltinTools.mdx

View workflow job for this annotation

GitHub Actions / build

The link for the `python_code_interpreter` tool needs to be updated to reflect the new tool name.

Choose a reason for hiding this comment

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

The link for the python_code_interpreter tool needs to be updated to reflect the new tool name.

generated by pr-docs-review-commit link_update

<LinkCard title="python_interpreter" description="Executes python 3.12 code in a docker container. The process output is returned. Use 'print' to output data." href="/genaiscript/reference/scripts/system#systempython_interpreter" />
<LinkCard title="python_code_interpreter" description="Executes python 3.12 code for Data Analysis tasks in a docker container. The process output is returned. Do not generate visualizations. The only packages available are numpy, pandas, scipy. There is NO network connectivity. Do not attempt to install other packages or make web requests." href="/genaiscript/reference/scripts/system#systempython_code_interpreter" />
<LinkCard title="retrieval_fuzz_search" description="Search for keywords using the full text of files and a fuzzy distance." href="/genaiscript/reference/scripts/system#systemretrieval_fuzz_search" />
<LinkCard title="retrieval_vector_search" description="Search files using embeddings and similarity distance." href="/genaiscript/reference/scripts/system#systemretrieval_vector_search" />
<LinkCard title="retrieval_web_search" description="Search the web for a user query using Bing Search." href="/genaiscript/reference/scripts/system#systemretrieval_web_search" />
Expand Down
43 changes: 16 additions & 27 deletions docs/src/content/docs/reference/scripts/system.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -590,64 +590,53 @@
`````


### `system.python_interpreter`
### `system.python_code_interpreter`

Check failure on line 593 in docs/src/content/docs/reference/scripts/system.mdx

View workflow job for this annotation

GitHub Actions / build

The header should be updated to `system.python_code_interpreter` to match the new tool name.

Choose a reason for hiding this comment

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

The header should be updated to system.python_code_interpreter to match the new tool name.

generated by pr-docs-review-commit header_update


Python Dockerized code execution
Python Dockerized code execution for data analysis



Check failure on line 598 in docs/src/content/docs/reference/scripts/system.mdx

View workflow job for this annotation

GitHub Actions / build

The description of the tool should be updated to "Executes python 3.12 code for Data Analysis tasks in a docker container."

Choose a reason for hiding this comment

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

The description of the tool should be updated to "Executes python 3.12 code for Data Analysis tasks in a docker container."

generated by pr-docs-review-commit description_update

- tool `python_interpreter`: Executes python 3.12 code in a docker container. The process output is returned. Use 'print' to output data.
- tool `python_code_interpreter`: Executes python 3.12 code for Data Analysis tasks in a docker container. The process output is returned. Do not generate visualizations. The only packages available are numpy, pandas, scipy. There is NO network connectivity. Do not attempt to install other packages or make web requests.

`````js wrap title="system.python_interpreter"
`````js wrap title="system.python_code_interpreter"
system({
title: "Python Dockerized code execution",
title: "Python Dockerized code execution for data analysis",
})

const image = env.vars.pythonImage ?? "python:3.12"
const packages = ["numpy", "pandas", "scipy"]

Check warning on line 607 in docs/src/content/docs/reference/scripts/system.mdx

View workflow job for this annotation

GitHub Actions / build

The declaration of the `packages` constant should be documented, explaining that only numpy, pandas, and scipy are available and no additional packages can be installed.

Choose a reason for hiding this comment

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

The declaration of the packages constant should be documented, explaining that only numpy, pandas, and scipy are available and no additional packages can be installed.

generated by pr-docs-review-commit packages_declaration


let container = null

defTool(

Check failure on line 611 in docs/src/content/docs/reference/scripts/system.mdx

View workflow job for this annotation

GitHub Actions / build

The tool name in the definition should be updated to `python_code_interpreter`.

Choose a reason for hiding this comment

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

The tool name in the definition should be updated to python_code_interpreter.

generated by pr-docs-review-commit tool_name_update

"python_interpreter",
"Executes python 3.12 code in a docker container. The process output is returned. Use 'print' to output data.",
"python_code_interpreter",
"Executes python 3.12 code for Data Analysis tasks in a docker container. The process output is returned. Do not generate visualizations. The only packages available are numpy, pandas, scipy. There is NO network connectivity. Do not attempt to install other packages or make web requests.",

Choose a reason for hiding this comment

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

The tool description should be updated to match the new functionality and limitations of the python_code_interpreter.

generated by pr-docs-review-commit tool_description_update

{
type: "object",
properties: {
requirements: {
type: "string",
description: `list of pip packages to install using pip. should be using the pip install format:
<package1>
<package2>
`,
},
main: {
type: "string",
description: "python 3.12 source code to execute",
},
},

Check failure on line 621 in docs/src/content/docs/reference/scripts/system.mdx

View workflow job for this annotation

GitHub Actions / build

The `requirements` property has been removed and should be updated in the documentation.

Choose a reason for hiding this comment

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

The requirements property has been removed and should be updated in the documentation.

generated by pr-docs-review-commit property_removal

Choose a reason for hiding this comment

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

The requirements field is no longer required and should be removed from the required fields list.

generated by pr-docs-review-commit required_fields_update

required: ["requirements", "main"],
required: ["main"],

Check failure on line 622 in docs/src/content/docs/reference/scripts/system.mdx

View workflow job for this annotation

GitHub Actions / build

The `required` array should be updated to only include `main` since `requirements` has been removed.

Choose a reason for hiding this comment

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

The required array should be updated to only include main since requirements has been removed.

generated by pr-docs-review-commit required_update

},
async (args) => {

Choose a reason for hiding this comment

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

The handling of the requirements field should be removed as it is no longer part of the tool's functionality.

generated by pr-docs-review-commit remove_requirements_handling

const { requirements, main = "" } = args
console.log(`python: running code...`)
container = await host.container({ image, networkEnabled: true })
if (requirements) {
console.log(`installing: ` + requirements)
await container.writeText(
"requirements.txt",
requirements.replace(/[ ,]\s*/g, "\n")
)
const { main = "" } = args
console.log(`python code interpreter: ` + main)
if (!container) {
console.log(`python: preparing container...`)
container = await host.container({ image, networkEnabled: true })
const res = await container.exec("pip", [
"install",
"--root-user-action",
"ignore",
"-r",
"requirements.txt",
...packages,
])
if (res.failed) throw new Error(`Failed to install requirements`)
await container.disconnect()

Check failure on line 637 in docs/src/content/docs/reference/scripts/system.mdx

View workflow job for this annotation

GitHub Actions / build

The section handling the installation of requirements has been removed and should be updated in the documentation.

Check failure on line 637 in docs/src/content/docs/reference/scripts/system.mdx

View workflow job for this annotation

GitHub Actions / build

The addition of `await container.disconnect()` should be documented.

Choose a reason for hiding this comment

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

The section handling the installation of requirements has been removed and should be updated in the documentation.

generated by pr-docs-review-commit installation_removal

Choose a reason for hiding this comment

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

The addition of await container.disconnect() should be documented.

generated by pr-docs-review-commit disconnect_addition

Choose a reason for hiding this comment

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

The call to container.disconnect() should be removed as it may interfere with the execution of the Python code in the container. Containers are typically disconnected after the execution is complete, not before.

generated by pr-docs-review-commit disconnect_removal

}

console.log(`code: ` + main)
await container.writeText("main.py", main)
const res = await container.exec("python", ["main.py"])
return res
Expand Down
13 changes: 9 additions & 4 deletions genaisrc/genaiscript.d.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 17 additions & 0 deletions packages/cli/src/docker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -299,17 +299,34 @@
}
}

const disconnect = async () => {
const networks = await this._docker.listNetworks()
for (const network of networks.filter(
({ Name }) => Name === "bridge"
)) {
const n = await this._docker.getNetwork(network.Id)
if (n) {
const state = await n.inspect()
if (state?.Containers?.[container.id]) {
console.log(`container: disconnect ${network.Name}`)

Check failure on line 311 in packages/cli/src/docker.ts

View workflow job for this annotation

GitHub Actions / build

There is a `console.log` statement on line 311. This could potentially leak sensitive information in a production environment. Consider using a proper logging mechanism that respects different log levels. 😊
pelikhan marked this conversation as resolved.
Show resolved Hide resolved
await n.disconnect({ Container: container.id })
}
}
}

Check failure on line 315 in packages/cli/src/docker.ts

View workflow job for this annotation

GitHub Actions / build

There is no error handling for the async operations within the `disconnect` function. If any of these operations fail, it could lead to unexpected behavior. Consider adding try-catch blocks to handle potential errors. 😊
pelikhan marked this conversation as resolved.
Show resolved Hide resolved
}

const c = <ContainerHost>{
id: container.id,
disablePurge: !!options.disablePurge,
hostPath,
containerPath,
stop,
exec,
writeText,
readText,
copyTo,
disconnect,
}

Check failure on line 329 in packages/cli/src/docker.ts

View workflow job for this annotation

GitHub Actions / build

The `disconnect` function is added to the `ContainerHost` object without any comments. It would be helpful to add a comment explaining what this function does for better code readability and maintainability. 😊
pelikhan marked this conversation as resolved.
Show resolved Hide resolved

Choose a reason for hiding this comment

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

The disconnect function is declared but not used within the DockerManager class. If this function is not needed, consider removing it to avoid confusion. If it is needed in the future, it can be added back. 😊

generated by pr-review-commit disconnect_unused

this.containers.push(c)
await container.start()

Expand Down
13 changes: 9 additions & 4 deletions packages/core/src/genaisrc/genaiscript.d.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

43 changes: 43 additions & 0 deletions packages/core/src/genaisrc/system.python_code_interpreter.genai.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
system({
title: "Python Dockerized code execution for data analysis",
})

const image = env.vars.pythonImage ?? "python:3.12"
const packages = ["numpy", "pandas", "scipy"]

let container = null

defTool(
"python_code_interpreter",
"Executes python 3.12 code for Data Analysis tasks in a docker container. The process output is returned. Do not generate visualizations. The only packages available are numpy, pandas, scipy. There is NO network connectivity. Do not attempt to install other packages or make web requests.",
{
type: "object",
properties: {
main: {
type: "string",
description: "python 3.12 source code to execute",
},
},
required: ["main"],
},
async (args) => {
const { main = "" } = args
console.log(`python code interpreter: ` + main)
if (!container) {
console.log(`python: preparing container...`)
container = await host.container({ image, networkEnabled: true })
const res = await container.exec("pip", [
"install",
"--root-user-action",
"ignore",
...packages,
])
if (res.failed) throw new Error(`Failed to install requirements`)
await container.disconnect()
}

await container.writeText("main.py", main)
const res = await container.exec("python", ["main.py"])
return res
}
)
54 changes: 0 additions & 54 deletions packages/core/src/genaisrc/system.python_interpreter.genai.js

This file was deleted.

5 changes: 5 additions & 0 deletions packages/core/src/types/prompt_template.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1594,6 +1594,11 @@ interface ContainerHost extends ShellHost {
* Stops and cleans out the container
*/
stop(): Promise<void>

/**
* Force disconnect network
*/
disconnect(): Promise<void>

Choose a reason for hiding this comment

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

The disconnect function in the ContainerHost interface is missing a detailed description. Consider adding more information about what the function does, what parameters it takes (if any), and what it returns. This will help other developers understand its purpose more easily. 😊

generated by pr-review-commit missing_function_description

}

interface PromptContext extends ChatGenerationContext {
Expand Down
Loading
Loading