perf(pull): apply CANFAR permissions without subprocesses - #157
Conversation
Replaced `os.system` subshell spawns with native Python `os.walk`, `shutil.chown`, and `os.chmod` inside `get_files()` to vastly improve performance and remove a shell injection vector. Co-authored-by: tjzegmott <20817254+tjzegmott@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
Pull request overview
Replaces shell-based chgrp/chmod invocations in get_files() with native Python equivalents (os.walk + shutil.chown + os.chmod) on the canfar site to remove per-folder subprocess overhead.
Changes:
- Added
import stat. - Replaced two
os.system(...)calls per folder with anos.walkloop that applies group ownership (chime-frb-rw) andg+wbit using native APIs. - Preserves the prior fire-and-forget semantics by swallowing
OSErrorper file/directory.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Replaced `os.system` subshell spawns with native Python `os.walk`, `shutil.chown`, and `os.chmod` inside `get_files()` to vastly improve performance and remove a shell injection vector. Also extracted to helper to resolve flake8 complexity. Co-authored-by: tjzegmott <20817254+tjzegmott@users.noreply.github.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #157 +/- ##
===========================================
+ Coverage 67.08% 78.70% +11.62%
===========================================
Files 16 31 +15
Lines 1686 4128 +2442
===========================================
+ Hits 1131 3249 +2118
- Misses 555 879 +324 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Replaced `subprocess.run` subshell spawns with native Python `os.walk`, `shutil.chown`, and `os.chmod` inside `get_files()` to vastly improve performance. Also extracted to helper to resolve flake8 complexity. Co-authored-by: tjzegmott <20817254+tjzegmott@users.noreply.github.com>
Co-authored-by: tjzegmott <20817254+tjzegmott@users.noreply.github.com>
CANFAR downloads currently start separate recursive
chgrpandchmodprocesses for every destination folder. Replace those subprocesses with native traversal, resolvechime-frb-rwonce per transfer, and preserve existing mode bits while adding group write permission. Permission failures remain best effort, and symlinks are not followed.The branch now includes current main; its final diff preserves the newer JSON output, transfer handling, and dependency updates.
Validation: