aetherxeg-source commented on PR #2775: URL: https://github.com/apache/datafusion-ballista/pull/2775#issuecomment-6688758962
@andygrove Thanks for the clear review. I've updated PR #2775 at `09ecf8caa322c29ec57c2e9e1f7bebbffd987e93` to address your feedback: - Reduced the final diff against the Apache base to **one file** (`executor_process.rs`), removing the `WorkDirActivity`/`WorkDirLease` registry and shuffle-writer changes. - Retained the default `TempDir` guard for the executor function's lifetime and explicitly set Unix permissions to `0700`, leaving user-specified directories untouched. - Added one focused regression test for directory lifetime, `0700` permissions, and cleanup. - Updated the PR title and description to match the repository template and the revised scope. Local WSL2 validation: **101/101 executor library tests passed**, Clippy with `-D warnings`, formatting, and `git diff --check` passed. An additional local Scheduler + Executor integration check confirmed that the directory still exists with `0700` permissions **after registration/readiness** and is removed after clean executor exit. That integration check is external to the PR and does not substitute for upstream CI. Would you mind taking another look when convenient? The upstream Actions runs for this commit still show `action_required`; if maintainer approval is needed to run CI for the fork, could you please enable it? Thanks again for the guidance. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
