Reduce the amount of data copied to the workflow pod#293
Conversation
There was a problem hiding this comment.
Pull request overview
This PR optimizes data copying to workflow pods by implementing a selective merge strategy instead of bulk copying all temporary files. The changes introduce a staging directory approach where only modified _runner_file_commands are overwritten and other files are appended only if missing, reducing unnecessary data transfer.
Key changes:
- Implemented a two-stage copy process with a temporary staging directory (
/_temp_pre) that merges into the final directory (/_temp) - Enhanced
fixArgs()to preserve shell command arguments forsh -cpatterns without re-tokenizing - Removed unnecessary quote wrapping from shell commands passed to
execPodStep
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/k8s/src/hooks/run-script-step.ts | Implements new merge strategy with staging directory and selective file copying to reduce data transfer |
| packages/k8s/src/k8s/utils.ts | Adds special handling in fixArgs to preserve sh -c command arguments without re-tokenizing |
| packages/k8s/src/k8s/index.ts | Removes unnecessary quotes from Alpine detection shell command |
| packages/k8s/tests/prepare-job-test.ts | Updates test to use recursive directory creation and removes unnecessary quote wrapping |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // - Append files not already present elsewhere | ||
| const mergeCommands = [ | ||
| 'set -e', | ||
| 'mkdir -p /__w/_temp /__w/_temp_pre', |
There was a problem hiding this comment.
The directories /__w/_temp and /__w/_temp_pre are already created in lines 30-39 before this merge command runs. The mkdir -p on this line is redundant and can be removed to simplify the code.
Suggested change:
'set -e',
'SRC=/__w/_temp_pre',
'DST=/__w/_temp',| 'mkdir -p /__w/_temp /__w/_temp_pre', |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Fixes #274