Skip to content

fix(entrypoint): create dest_dir before download_distribution for nix containers#31

Open
devin-ai-integration[bot] wants to merge 1 commit intomasterfrom
devin/1770708453-fix-dest-dir-mkdir
Open

fix(entrypoint): create dest_dir before download_distribution for nix containers#31
devin-ai-integration[bot] wants to merge 1 commit intomasterfrom
devin/1770708453-fix-dest-dir-mkdir

Conversation

@devin-ai-integration
Copy link

Why are the changes needed?

Nix-built container images (ImageSpec(nix=True)) produce minimal filesystem layouts that may not include /root. The pyflyte register CLI defaults --destination-dir to /root, which gets baked into the task template as --dest-dir /root. When pyflyte-fast-execute runs inside a nix container, download_distribution raises:

ValueError: Destination path is required to download distribution and it should be a directory

because /root doesn't exist in the container (nix images use WorkingDir=/service).

Observed in production on: flytesnacks/staging/ax9ttpbwtd6crjxdqdfz

What changes were proposed in this pull request?

Add os.makedirs(dest_dir, exist_ok=True) in fast_execute_task_cmd before calling _download_distribution. This ensures the destination directory is created if it doesn't already exist, making the entrypoint resilient to minimal container filesystems.

How was this patch tested?

Not yet tested with a unit test. The fix was validated by tracing the exact failure path from production Loki logs through the entrypoint code. The exist_ok=True flag ensures no behavior change when the directory already exists (i.e., all non-nix containers).

Check all the applicable boxes

  • I updated the documentation accordingly.
  • All new and existing tests passed.
  • All commits are signed-off.

Human Review Checklist

  • Confirm os.makedirs won't hit permission issues in restricted containers
  • Consider whether silently creating /root could mask a misconfigured --destination-dir

Link to Devin run | Requested by @sammiphone6

… containers

Co-Authored-By: Samuel Mitchell <sammiphone6@gmail.com>
@devin-ai-integration
Copy link
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant