-
-
Notifications
You must be signed in to change notification settings - Fork 641
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(build): Add support for Python scripts via pythonExtension
#1686
base: main
Are you sure you want to change the base?
Changes from all commits
2a4aa8c
987ab35
8b97cec
84699e5
d00d147
500d82b
f0b30d6
b63554e
8ad7009
c3cbd61
be2cefe
bb59bf8
65d84d0
7c96930
c599809
015f3aa
4302077
8ef78ee
ca51709
ddc706a
311aed3
0e2d2e5
cf95fca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"@trigger.dev/build": minor | ||
--- | ||
|
||
Introduced a new Python extension to enhance the build process. It now allows users to execute Python scripts with improved support and error handling. |
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,175 @@ | ||||||||||||
import fs from "node:fs"; | ||||||||||||
import assert from "node:assert"; | ||||||||||||
import { additionalFiles } from "./core/additionalFiles.js"; | ||||||||||||
import { BuildManifest } from "@trigger.dev/core/v3"; | ||||||||||||
import { BuildContext, BuildExtension } from "@trigger.dev/core/v3/build"; | ||||||||||||
import { logger } from "@trigger.dev/sdk/v3"; | ||||||||||||
import { x, Options as XOptions, Result } from "tinyexec"; | ||||||||||||
|
||||||||||||
export type PythonOptions = { | ||||||||||||
requirements?: string[]; | ||||||||||||
requirementsFile?: string; | ||||||||||||
/** | ||||||||||||
* [Dev-only] The path to the python binary. | ||||||||||||
* | ||||||||||||
* @remarks | ||||||||||||
* This option is typically used during local development or in specific testing environments | ||||||||||||
* where a particular Python installation needs to be targeted. It should point to the full path of the python executable. | ||||||||||||
* | ||||||||||||
* Example: `/usr/bin/python3` or `C:\\Python39\\python.exe` | ||||||||||||
*/ | ||||||||||||
pythonBinaryPath?: string; | ||||||||||||
/** | ||||||||||||
* An array of glob patterns that specify which Python scripts are allowed to be executed. | ||||||||||||
* | ||||||||||||
* @remarks | ||||||||||||
* These scripts will be copied to the container during the build process. | ||||||||||||
*/ | ||||||||||||
scripts?: string[]; | ||||||||||||
}; | ||||||||||||
|
||||||||||||
const splitAndCleanComments = (str: string) => | ||||||||||||
str | ||||||||||||
.split("\n") | ||||||||||||
.map((line) => line.trim()) | ||||||||||||
.filter((line) => line && !line.startsWith("#")); | ||||||||||||
|
||||||||||||
export function pythonExtension(options: PythonOptions = {}): BuildExtension { | ||||||||||||
return new PythonExtension(options); | ||||||||||||
} | ||||||||||||
|
||||||||||||
class PythonExtension implements BuildExtension { | ||||||||||||
public readonly name = "PythonExtension"; | ||||||||||||
|
||||||||||||
constructor(private options: PythonOptions = {}) { | ||||||||||||
assert( | ||||||||||||
!(this.options.requirements && this.options.requirementsFile), | ||||||||||||
"Cannot specify both requirements and requirementsFile" | ||||||||||||
); | ||||||||||||
|
||||||||||||
if (this.options.requirementsFile) { | ||||||||||||
assert( | ||||||||||||
fs.existsSync(this.options.requirementsFile), | ||||||||||||
`Requirements file not found: ${this.options.requirementsFile}` | ||||||||||||
); | ||||||||||||
this.options.requirements = splitAndCleanComments( | ||||||||||||
fs.readFileSync(this.options.requirementsFile, "utf-8") | ||||||||||||
); | ||||||||||||
} | ||||||||||||
} | ||||||||||||
|
||||||||||||
async onBuildComplete(context: BuildContext, manifest: BuildManifest) { | ||||||||||||
await additionalFiles({ | ||||||||||||
files: this.options.scripts ?? [], | ||||||||||||
}).onBuildComplete!(context, manifest); | ||||||||||||
|
||||||||||||
if (context.target === "dev") { | ||||||||||||
if (this.options.pythonBinaryPath) { | ||||||||||||
process.env.PYTHON_BIN_PATH = this.options.pythonBinaryPath; | ||||||||||||
} | ||||||||||||
|
||||||||||||
return; | ||||||||||||
} | ||||||||||||
|
||||||||||||
context.logger.debug(`Adding ${this.name} to the build`); | ||||||||||||
|
||||||||||||
context.addLayer({ | ||||||||||||
id: "python-installation", | ||||||||||||
image: { | ||||||||||||
instructions: splitAndCleanComments(` | ||||||||||||
# Install Python | ||||||||||||
RUN apt-get update && apt-get install -y --no-install-recommends \ | ||||||||||||
python3 python3-pip python3-venv && \ | ||||||||||||
apt-get clean && rm -rf /var/lib/apt/lists/* | ||||||||||||
zvictor marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||
|
||||||||||||
# Set up Python environment | ||||||||||||
RUN python3 -m venv /opt/venv | ||||||||||||
ENV PATH="/opt/venv/bin:$PATH" | ||||||||||||
`), | ||||||||||||
}, | ||||||||||||
deploy: { | ||||||||||||
env: { | ||||||||||||
PYTHON_BIN_PATH: `/opt/venv/bin/python`, | ||||||||||||
}, | ||||||||||||
override: true, | ||||||||||||
}, | ||||||||||||
}); | ||||||||||||
|
||||||||||||
context.addLayer({ | ||||||||||||
id: "python-dependencies", | ||||||||||||
build: { | ||||||||||||
env: { | ||||||||||||
REQUIREMENTS_CONTENT: this.options.requirements?.join("\n") || "", | ||||||||||||
}, | ||||||||||||
}, | ||||||||||||
image: { | ||||||||||||
instructions: splitAndCleanComments(` | ||||||||||||
ARG REQUIREMENTS_CONTENT | ||||||||||||
RUN echo "$REQUIREMENTS_CONTENT" > requirements.txt | ||||||||||||
|
||||||||||||
# Install dependencies | ||||||||||||
RUN pip install --no-cache-dir -r requirements.txt | ||||||||||||
`), | ||||||||||||
zvictor marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||
}, | ||||||||||||
deploy: { | ||||||||||||
override: true, | ||||||||||||
}, | ||||||||||||
}); | ||||||||||||
} | ||||||||||||
} | ||||||||||||
|
||||||||||||
export const run = async ( | ||||||||||||
scriptArgs: string[] = [], | ||||||||||||
options: Partial<XOptions> = {} | ||||||||||||
): Promise<Result> => { | ||||||||||||
const pythonBin = process.env.PYTHON_BIN_PATH || "python"; | ||||||||||||
|
||||||||||||
const result = await x(pythonBin, scriptArgs, { | ||||||||||||
...options, | ||||||||||||
throwOnError: false, // Ensure errors are handled manually | ||||||||||||
}); | ||||||||||||
|
||||||||||||
try { | ||||||||||||
assert( | ||||||||||||
result.exitCode === 0, | ||||||||||||
`Python command exited with non-zero code ${result.exitCode}\nStdout: ${result.stdout}\nStderr: ${result.stderr}` | ||||||||||||
); | ||||||||||||
} catch (error) { | ||||||||||||
logger.error("Python command execution failed", { | ||||||||||||
error: error instanceof Error ? error.message : error, | ||||||||||||
command: `${pythonBin} ${scriptArgs.join(" ")}`, | ||||||||||||
stdout: result.stdout, | ||||||||||||
stderr: result.stderr, | ||||||||||||
exitCode: result.exitCode, | ||||||||||||
}); | ||||||||||||
throw error; | ||||||||||||
} | ||||||||||||
|
||||||||||||
return result; | ||||||||||||
}; | ||||||||||||
|
||||||||||||
export const runScript = ( | ||||||||||||
scriptPath: string, | ||||||||||||
scriptArgs: string[] = [], | ||||||||||||
options: Partial<XOptions> = {} | ||||||||||||
) => { | ||||||||||||
assert(scriptPath, "Script path is required"); | ||||||||||||
assert(fs.existsSync(scriptPath), `Script does not exist: ${scriptPath}`); | ||||||||||||
Comment on lines
+156
to
+157
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TODO
trigger.dev/packages/build/src/extensions/core/additionalFiles.ts Lines 37 to 41 in bc7d445
Questions
Comment on lines
+156
to
+157
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainAddress the TODO comment about script pattern validation. As noted in the TODO comment, we need to implement the fail-fast approach for script pattern validation in development. Let's verify the current behavior and propose a solution: 🏁 Script executed: #!/bin/bash
# Search for any existing pattern matching utilities
rg -A 5 "doesMatchPattern|findStaticAssetFiles" . Length of output: 1173 Implement the script pattern validation check in development The search confirms that the functionality to validate script patterns (using a fail-fast approach during development) hasn’t been implemented in the Python extension. To address the TODO, consider adding a validation function similar to the following:
Example implementation snippet: // packages/build/src/extensions/python.ts
import * as path from 'path';
/**
* Validates that the script matches the expected Python script pattern.
* @param scriptPath The path of the script.
*/
function validateScriptPattern(scriptPath: string) {
const scriptName = path.basename(scriptPath);
// Define your expected pattern for Python scripts (customize as needed)
const pattern = /^[\w\-]+\.py$/;
if (!pattern.test(scriptName)) {
throw new Error(`Invalid script file name: ${scriptName}. Ensure it follows the expected naming convention.`);
}
}
// Existing assertions
assert(scriptPath, "Script path is required");
assert(fs.existsSync(scriptPath), `Script does not exist: ${scriptPath}`);
// In development, validate the script pattern early
if (process.env.NODE_ENV === 'development') {
validateScriptPattern(scriptPath);
} This approach ensures that any deviation from the expected script naming convention is caught early in the development phase, adhering to the fail-fast principle. |
||||||||||||
|
||||||||||||
return run([scriptPath, ...scriptArgs], options); | ||||||||||||
}; | ||||||||||||
|
||||||||||||
export const runInline = async (scriptContent: string, options: Partial<XOptions> = {}) => { | ||||||||||||
assert(scriptContent, "Script content is required"); | ||||||||||||
|
||||||||||||
const tmpFile = `/tmp/script_${Date.now()}.py`; | ||||||||||||
await fs.promises.writeFile(tmpFile, scriptContent, { mode: 0o600 }); | ||||||||||||
|
||||||||||||
try { | ||||||||||||
return await runScript(tmpFile, [], options); | ||||||||||||
} finally { | ||||||||||||
await fs.promises.unlink(tmpFile); | ||||||||||||
} | ||||||||||||
}; | ||||||||||||
|
||||||||||||
export default { run, runScript, runInline }; |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update pnpm-lock.yaml for new dependencies.
The pipeline is failing because the lock file hasn't been updated after adding new dependencies. Run
pnpm install
to update the lock file.Also applies to: 75-75