Add configurable remote Python path (fixes #314)#315
Conversation
- Add `remote_python_path` config field so users with custom Python installs on their seedbox can override the default `python3` - Make scan_fs.py compatible with Python 3.5+ (use typing.List/Optional instead of modern syntax, remove `from __future__ import annotations`) - Add Remote Python Path option to Settings UI (Server section) - Wire config through controller to RemoteScanner - Update README, docs, and FAQ with the new setting - Add ruff per-file ignores for scan_fs.py (Python 3.5 compat) - Fix test data for directory check guard in _install_scanfs Closes #314 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a configurable "Remote Python Path" setting, threads it through config and controller into RemoteScanner so remote scans invoke the specified Python binary (falling back to Changes
Sequence Diagram(s)sequenceDiagram
participant UI as "Web UI"
participant Config as "Config Layer"
participant Controller as "Controller"
participant Scanner as "RemoteScanner"
participant SSH as "SSH Client"
participant Remote as "Remote Host"
UI->>Config: user sets Remote Python Path
Controller->>Config: read lftp.remote_python_path
Controller->>Scanner: init(..., remote_python_path)
Scanner->>SSH: ssh.shell(run "<remote_python_cmd> /tmp/scan_fs.py ...")
SSH->>Remote: execute command
Remote-->>SSH: command output
SSH-->>Scanner: result
Scanner-->>Controller: scan result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/python/tests/unittests/test_common/test_config.py (1)
546-593: 🧹 Nitpick | 🔵 TrivialAdd one non-empty
remote_python_pathround-trip assertion.This change covers the empty case well; please also add a non-empty path check (e.g.,
/opt/python/bin/python3) to guard serialization/deserialization regressions for configured values.✅ Minimal test extension
- config.lftp.remote_python_path = "" + config.lftp.remote_python_path = "/opt/python/bin/python3" ... - remote_python_path = + remote_python_path = /opt/python/bin/python3🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/python/tests/unittests/test_common/test_config.py` around lines 546 - 593, Set a non-empty remote_python_path before serializing and assert it round-trips: change the assignment config.lftp.remote_python_path = "" to config.lftp.remote_python_path = "/opt/python/bin/python3", update the expected golden block to include the line remote_python_path = /opt/python/bin/python3, and ensure the test still calls config.to_file(config_file_path) and reads actual_str to assert the serialized file contains that non-empty path (use the existing config_file_path, config.to_file and actual_str variables).src/python/common/config.py (1)
257-291:⚠️ Potential issue | 🟡 MinorAdd
.strip()to normalizeremote_python_pathbefore use.The config definition is correct, but downstream in
src/python/controller/scan/remote_scanner.py(line 42), the parameter is stored without trimming whitespace. The checkif remote_python_path else "python3"only guards against empty strings—whitespace-only input (e.g., spaces) would pass through and create malformed commands. Add.strip()when assigning:self.__remote_python_cmd = (remote_python_path.strip() if remote_python_path else "python3")to handle this edge case.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/python/common/config.py` around lines 257 - 291, The remote Python command can be a whitespace-only string because remote_python_path isn't trimmed before use; update the RemoteScanner (or the code that sets __remote_python_cmd) to use remote_python_path.strip() when building the command — e.g., compute __remote_python_cmd as (remote_python_path.strip() if remote_python_path else "python3") so whitespace-only values fall back to "python3".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/python/controller/scan/remote_scanner.py`:
- Around line 86-98: The constructed shell commands insert
self.__remote_python_cmd unescaped which will break for paths with
spaces/special chars; update both branches that call self.__ssh.shell to wrap
__remote_python_cmd with the same escaping used for script/scan paths (use
_escape_remote_path_double in the first branch and _escape_remote_path_single in
the else branch) so the full command string uses escaped Python path along with
the escaped script and scan paths.
In `@src/python/scan_fs.py`:
- Around line 189-190: Update the Python version guard that currently checks
sys.hexversion in scan_fs.py: change the numeric literal from 0x03050000 to
0x030C0000 and update the sys.exit message string to "Python 3.12 or newer is
required to run this program." so the check in the top-level version validation
uses Python 3.12 as the minimum.
---
Outside diff comments:
In `@src/python/common/config.py`:
- Around line 257-291: The remote Python command can be a whitespace-only string
because remote_python_path isn't trimmed before use; update the RemoteScanner
(or the code that sets __remote_python_cmd) to use remote_python_path.strip()
when building the command — e.g., compute __remote_python_cmd as
(remote_python_path.strip() if remote_python_path else "python3") so
whitespace-only values fall back to "python3".
In `@src/python/tests/unittests/test_common/test_config.py`:
- Around line 546-593: Set a non-empty remote_python_path before serializing and
assert it round-trips: change the assignment config.lftp.remote_python_path = ""
to config.lftp.remote_python_path = "/opt/python/bin/python3", update the
expected golden block to include the line remote_python_path =
/opt/python/bin/python3, and ensure the test still calls
config.to_file(config_file_path) and reads actual_str to assert the serialized
file contains that non-empty path (use the existing config_file_path,
config.to_file and actual_str variables).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: dfe317df-84db-4f72-af9c-602355acfe7f
📒 Files selected for processing (11)
README.mdsrc/angular/src/app/pages/settings/options-list.tssrc/python/common/config.pysrc/python/controller/controller.pysrc/python/controller/scan/remote_scanner.pysrc/python/pyproject.tomlsrc/python/scan_fs.pysrc/python/tests/unittests/test_common/test_config.pysrc/python/tests/unittests/test_controller/test_scan/test_remote_scanner.pywebsite/docs/configuration.mdwebsite/docs/faq.md
| if sys.hexversion < 0x03050000: | ||
| sys.exit("Python 3.5 or newer is required to run this program.") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify no Python 3.6+ specific syntax remains in scan_fs.py
# Check for f-strings (3.6+), walrus operator (3.8+), or union types (3.10+)
echo "Checking for f-strings (Python 3.6+):"
rg -n 'f"[^"]*\{' src/python/scan_fs.py || echo "No f-strings found"
rg -n "f'[^']*\{" src/python/scan_fs.py || echo "No f-strings found"
echo ""
echo "Checking for walrus operator := (Python 3.8+):"
rg -n ':=' src/python/scan_fs.py || echo "No walrus operators found"
echo ""
echo "Checking for union type syntax | (Python 3.10+):"
rg -n '\w+\s*\|\s*\w+' src/python/scan_fs.py | grep -v '#' || echo "No union types found"Repository: nitrobass24/seedsync
Length of output: 284
🏁 Script executed:
cd src/python && head -200 scan_fs.py | tail -20Repository: nitrobass24/seedsync
Length of output: 844
Version check must be updated to Python 3.12 to comply with coding guidelines.
The file src/python/scan_fs.py matches the pattern src/python/**/*.py, which requires Python 3.12 per project guidelines. The current check if sys.hexversion < 0x03050000: must be changed to 0x030C0000 (Python 3.12). Update the error message accordingly to "Python 3.12 or newer is required to run this program."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/python/scan_fs.py` around lines 189 - 190, Update the Python version
guard that currently checks sys.hexversion in scan_fs.py: change the numeric
literal from 0x03050000 to 0x030C0000 and update the sys.exit message string to
"Python 3.12 or newer is required to run this program." so the check in the
top-level version validation uses Python 3.12 as the minimum.
- Escape __remote_python_cmd with the same quoting used for script/scan paths so paths with spaces or special chars work correctly - Strip whitespace from remote_python_path so whitespace-only values fall back to "python3" - Use non-empty remote_python_path in config round-trip test Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/python/tests/unittests/test_controller/test_scan/test_remote_scanner.py`:
- Around line 478-501: The command-building logic in RemoteScanner
(remote_scanner.py) currently chooses quoting based solely on
remote_path_to_scan and thus single-quotes the entire command when the scan path
is absolute, preventing tilde expansion in remote_python_path (e.g.
"~/python3/bin/python3"); modify the code that constructs the shell command (the
Scan flow / scan() or the helper that formats arguments) to decide quoting per
argument: if an argument starts with '~' do not wrap it in single quotes (or
wrap that arg in double quotes) so the shell can expand the tilde, otherwise
keep existing quoting behavior; ensure remote_python_path and
remote_path_to_scan are handled independently when building the final command
string passed to self.mock_ssh.shell.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: cf13c14e-4c65-4efc-849a-70005bbbb7c4
📒 Files selected for processing (3)
src/python/controller/scan/remote_scanner.pysrc/python/tests/unittests/test_common/test_config.pysrc/python/tests/unittests/test_controller/test_scan/test_remote_scanner.py
| def test_uses_custom_python_path(self): | ||
| """When remote_python_path is set, uses it instead of 'python3'""" | ||
| scanner = RemoteScanner( | ||
| remote_address="my remote address", | ||
| remote_username="my remote user", | ||
| remote_password="my password", | ||
| remote_port=1234, | ||
| remote_path_to_scan="/remote/path/to/scan", | ||
| local_path_to_scan_script=TestRemoteScanner.temp_scan_script, | ||
| remote_path_to_scan_script="/remote/path/to/scan/script", | ||
| remote_python_path="~/python3/bin/python3", | ||
| ) | ||
|
|
||
| self.mock_ssh.shell.side_effect = self._make_shell_side_effect( | ||
| [ | ||
| b"d41d8cd98f00b204e9800998ecf8427e", # md5sum - matches, skip install | ||
| json.dumps([]).encode(), # scanfs | ||
| ] | ||
| ) | ||
|
|
||
| scanner.scan() | ||
| self.mock_ssh.shell.assert_called_with( | ||
| "'~/python3/bin/python3' '/remote/path/to/scan/script' '/remote/path/to/scan'" | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify how tilde is handled in the remote_python_path configuration documentation
rg -n -C3 'remote_python_path' --type=mdRepository: nitrobass24/seedsync
Length of output: 1782
🏁 Script executed:
#!/bin/bash
# First, list files to understand structure
git ls-files "src/python/tests/unittests/test_controller/test_scan/" | head -20Repository: nitrobass24/seedsync
Length of output: 1885
🏁 Script executed:
#!/bin/bash
# Find the RemoteScanner implementation
find . -type f -name "*.py" | xargs grep -l "class RemoteScanner" | head -5Repository: nitrobass24/seedsync
Length of output: 110
🏁 Script executed:
#!/bin/bash
# Try to read the test file directly
cat -n "src/python/tests/unittests/test_controller/test_scan/test_remote_scanner.py" | sed -n '478,520p'Repository: nitrobass24/seedsync
Length of output: 2089
🏁 Script executed:
#!/bin/bash
# Read RemoteScanner implementation - focusing on scan method
cat -n "src/python/controller/scan/remote_scanner.py" | head -150Repository: nitrobass24/seedsync
Length of output: 7667
🏁 Script executed:
#!/bin/bash
# Search for the scan method and command construction in RemoteScanner
rg -n "def scan" "src/python/controller/scan/remote_scanner.py" -A 50Repository: nitrobass24/seedsync
Length of output: 2532
🏁 Script executed:
#!/bin/bash
# Check for other tests related to python path with different scan paths
rg -n "remote_python_path" "src/python/tests/unittests/test_controller/test_scan/test_remote_scanner.py" -B 3 -A 3Repository: nitrobass24/seedsync
Length of output: 1023
🏁 Script executed:
#!/bin/bash
# Check documentation or comments about tilde expansion
rg -n "tilde\|~" "src/python/controller/scan/remote_scanner.py" -B 1 -A 1Repository: nitrobass24/seedsync
Length of output: 46
Fix: Tilde in remote_python_path won't expand when scan path is absolute.
The implementation determines quoting strategy based only on whether remote_path_to_scan starts with ~ (lines 84-101 in remote_scanner.py). When the scan path is absolute but remote_python_path contains tilde (e.g., ~/bin/python3), single quotes are applied to all arguments. This prevents tilde expansion in the python path, causing execution to fail with "command ~ not found."
The test correctly reflects this behavior, but the behavior is a bug: users configuring remote_python_path="~/python3/bin/python3" with an absolute remote_path will encounter failures. The quoting logic should evaluate tilde expansion independently for each path argument, or consistently use double quotes when any argument requires expansion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/python/tests/unittests/test_controller/test_scan/test_remote_scanner.py`
around lines 478 - 501, The command-building logic in RemoteScanner
(remote_scanner.py) currently chooses quoting based solely on
remote_path_to_scan and thus single-quotes the entire command when the scan path
is absolute, preventing tilde expansion in remote_python_path (e.g.
"~/python3/bin/python3"); modify the code that constructs the shell command (the
Scan flow / scan() or the helper that formats arguments) to decide quoting per
argument: if an argument starts with '~' do not wrap it in single quotes (or
wrap that arg in double quotes) so the shell can expand the tilde, otherwise
keep existing quoting behavior; ensure remote_python_path and
remote_path_to_scan are handled independently when building the final command
string passed to self.mock_ssh.shell.
1 participant
Summary
scan_fs.pynow works on remote servers with Python 3.5+ by replacing modern type syntax (X | None,list[X]) withtyping.List/Optionaland removingfrom __future__ import annotationsRemote Python Pathsetting in the Server section lets users specify a custom Python binary on their seedbox (e.g.~/python3/bin/python3). Defaults topython3when empty.Changes
scan_fs.pytypingimports, quoted forward refs, lower version checkconfig.pyremote_python_pathproperty (empty string default)remote_scanner.pyremote_python_pathparam, use it instead of hardcodedpython3controller.pyremote_python_pathfrom config toRemoteScanneroptions-list.tspyproject.tomlscan_fs.py(UP006/UP035/UP045)test_config.pyremote_python_pathto golden config stringtest_remote_scanner.pyREADME.md,configuration.md,faq.mdTest plan
Closes #314
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
python3when empty).Documentation