-
Notifications
You must be signed in to change notification settings - Fork 5
Add configurable remote Python path (fixes #314) #315
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,17 +4,16 @@ | |
| # This script is uploaded to the remote server and executed standalone via | ||
| # `python3 scan_fs.py <path>`. It must NOT import any SeedSync packages. | ||
| # | ||
| # IMPORTANT: This file runs on the REMOTE server which may have Python 3.8+. | ||
| # Use `from __future__ import annotations` so modern type syntax (X | None, | ||
| # list[X]) is valid on older Python versions. | ||
|
|
||
| from __future__ import annotations | ||
| # IMPORTANT: This file runs on the REMOTE server which may have Python 3.5+. | ||
| # Do NOT use modern type syntax (X | None, list[X]) or | ||
| # `from __future__ import annotations` — use typing imports instead. | ||
|
|
||
| import json | ||
| import os | ||
| import re | ||
| import sys | ||
| from datetime import datetime | ||
| from typing import List, Optional | ||
|
|
||
|
|
||
| class SystemFile: | ||
|
|
@@ -29,8 +28,8 @@ def __init__( | |
| name: str, | ||
| size: int, | ||
| is_dir: bool = False, | ||
| time_created: datetime | None = None, | ||
| time_modified: datetime | None = None, | ||
| time_created: Optional[datetime] = None, | ||
| time_modified: Optional[datetime] = None, | ||
| ): | ||
| if size < 0: | ||
| raise ValueError("File size must be non-negative") | ||
|
|
@@ -54,10 +53,10 @@ def is_dir(self) -> bool: | |
| return self.__is_dir | ||
|
|
||
| @property | ||
| def children(self) -> list[SystemFile]: | ||
| def children(self) -> "List[SystemFile]": | ||
| return self.__children | ||
|
|
||
| def add_child(self, file: SystemFile): | ||
| def add_child(self, file: "SystemFile"): | ||
| if not self.__is_dir: | ||
| raise TypeError("Cannot add children to a file") | ||
| self.__children.append(file) | ||
|
|
@@ -96,7 +95,7 @@ def add_exclude_prefix(self, prefix: str): | |
| def add_exclude_suffix(self, suffix: str): | ||
| self.exclude_suffixes.append(suffix) | ||
|
|
||
| def scan(self) -> list[SystemFile]: | ||
| def scan(self) -> "List[SystemFile]": | ||
| if not os.path.exists(self.path_to_scan): | ||
| raise SystemScannerError("Path does not exist: {}".format(self.path_to_scan)) | ||
| elif not os.path.isdir(self.path_to_scan): | ||
|
|
@@ -133,7 +132,7 @@ def __create_system_file(self, entry) -> SystemFile: | |
| sys_file = SystemFile(file_name, file_size, False, time_created=time_created, time_modified=time_modified) | ||
| return sys_file | ||
|
|
||
| def __create_children(self, path: str) -> list[SystemFile]: | ||
| def __create_children(self, path: str) -> "List[SystemFile]": | ||
| children = [] | ||
| for entry in os.scandir(path): | ||
| skip = False | ||
|
|
@@ -187,8 +186,8 @@ def _lftp_status_file_size(status: str) -> int: | |
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| if sys.hexversion < 0x03060000: | ||
| sys.exit("Python 3.6 or newer is required to run this program.") | ||
| if sys.hexversion < 0x03050000: | ||
| sys.exit("Python 3.5 or newer is required to run this program.") | ||
|
Comment on lines
+189
to
+190
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 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 🤖 Prompt for AI Agents |
||
|
|
||
| import argparse | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -67,7 +67,8 @@ def _make_shell_side_effect(self, responses): | |
| Each entry is either a bytes value to return, or an Exception to raise. | ||
| The shell call sequence on first scan is: | ||
| 1. md5sum check | ||
| 2. scanfs (possibly retried) | ||
| 2. directory check (if md5sum doesn't match) | ||
| 3. scanfs (possibly retried) | ||
| """ | ||
| self._shell_call_index = 0 | ||
|
|
||
|
|
@@ -111,10 +112,11 @@ def mock_ssh_ctor(**kwargs): | |
| def test_installs_scan_script_on_first_scan(self): | ||
| scanner = self._make_scanner() | ||
|
|
||
| # Call sequence: md5sum (non-matching), scanfs | ||
| # Call sequence: md5sum (non-matching), directory check, scanfs | ||
| self.mock_ssh.shell.side_effect = self._make_shell_side_effect( | ||
| [ | ||
| b"", # md5sum - doesn't match, triggers install | ||
| b"OK", # directory check | ||
| json.dumps([]).encode(), # scanfs | ||
| json.dumps([]).encode(), # second scan: scanfs (no install) | ||
| ] | ||
|
|
@@ -136,6 +138,7 @@ def test_copy_appends_scanfs_name_to_remote_path(self): | |
| self.mock_ssh.shell.side_effect = self._make_shell_side_effect( | ||
| [ | ||
| b"", # md5sum - doesn't match | ||
| b"OK", # directory check | ||
| json.dumps([]).encode(), # scanfs | ||
| ] | ||
| ) | ||
|
|
@@ -152,13 +155,14 @@ def test_calls_correct_ssh_md5sum_command(self): | |
| self.mock_ssh.shell.side_effect = self._make_shell_side_effect( | ||
| [ | ||
| b"", # md5sum | ||
| b"OK", # directory check | ||
| json.dumps([]).encode(), # scanfs | ||
| ] | ||
| ) | ||
|
|
||
| scanner.scan() | ||
| # 2 calls: md5sum, scanfs | ||
| self.assertEqual(2, self.mock_ssh.shell.call_count) | ||
| # 3 calls: md5sum, directory check, scanfs | ||
| self.assertEqual(3, self.mock_ssh.shell.call_count) | ||
| # First call should be the md5sum command | ||
| md5sum_call = self.mock_ssh.shell.call_args_list[0] | ||
| self.assertEqual(call("md5sum '/remote/path/to/scan/script' | awk '{print $1}' || echo"), md5sum_call) | ||
|
|
@@ -189,6 +193,7 @@ def test_installs_scan_script_on_any_md5sum_output(self): | |
| self.mock_ssh.shell.side_effect = self._make_shell_side_effect( | ||
| [ | ||
| b"some output from md5sum", # md5sum - doesn't match | ||
| b"OK", # directory check | ||
| json.dumps([]).encode(), # scanfs | ||
| ] | ||
| ) | ||
|
|
@@ -218,14 +223,15 @@ def test_calls_correct_ssh_scan_command(self): | |
| self.mock_ssh.shell.side_effect = self._make_shell_side_effect( | ||
| [ | ||
| b"", # md5sum | ||
| b"OK", # directory check | ||
| json.dumps([]).encode(), # scanfs | ||
| ] | ||
| ) | ||
|
|
||
| scanner.scan() | ||
| # 2 calls: md5sum, scanfs | ||
| self.assertEqual(2, self.mock_ssh.shell.call_count) | ||
| self.mock_ssh.shell.assert_called_with("python3 '/remote/path/to/scan/script' '/remote/path/to/scan'") | ||
| # 3 calls: md5sum, directory check, scanfs | ||
| self.assertEqual(3, self.mock_ssh.shell.call_count) | ||
| self.mock_ssh.shell.assert_called_with("'python3' '/remote/path/to/scan/script' '/remote/path/to/scan'") | ||
|
|
||
| def test_handles_tilde_path_for_shell_expansion(self): | ||
| """Test that paths starting with ~ are converted to $HOME for shell expansion""" | ||
|
|
@@ -234,15 +240,16 @@ def test_handles_tilde_path_for_shell_expansion(self): | |
| self.mock_ssh.shell.side_effect = self._make_shell_side_effect( | ||
| [ | ||
| b"", # md5sum | ||
| b"OK", # directory check | ||
| json.dumps([]).encode(), # scanfs | ||
| ] | ||
| ) | ||
|
|
||
| scanner.scan() | ||
| self.assertEqual(2, self.mock_ssh.shell.call_count) | ||
| self.assertEqual(3, self.mock_ssh.shell.call_count) | ||
| # When scan path has tilde, both paths use double quotes for consistent quoting | ||
| # Tilde is converted to $HOME for shell expansion | ||
| self.mock_ssh.shell.assert_called_with('python3 "/remote/path/to/scan/script" "$HOME/data/torrents"') | ||
| self.mock_ssh.shell.assert_called_with('"python3" "/remote/path/to/scan/script" "$HOME/data/torrents"') | ||
|
|
||
| def test_raises_nonrecoverable_error_on_first_failed_ssh(self): | ||
| """Non-transient errors on first run are non-recoverable (no retry)""" | ||
|
|
@@ -251,6 +258,7 @@ def test_raises_nonrecoverable_error_on_first_failed_ssh(self): | |
| self.mock_ssh.shell.side_effect = self._make_shell_side_effect( | ||
| [ | ||
| b"", # md5sum | ||
| b"OK", # directory check | ||
| SshcpError("an ssh error"), # scanfs fails (non-transient) | ||
| ] | ||
| ) | ||
|
|
@@ -267,6 +275,7 @@ def test_raises_recoverable_error_on_subsequent_failed_ssh(self): | |
| self.mock_ssh.shell.side_effect = self._make_shell_side_effect( | ||
| [ | ||
| b"", # md5sum | ||
| b"OK", # directory check | ||
| json.dumps([]).encode(), # first scanfs - success | ||
| # second scan: 3 retry attempts all fail | ||
| SshcpError("an ssh error"), | ||
|
|
@@ -288,6 +297,7 @@ def test_recovers_from_failed_ssh(self): | |
| self.mock_ssh.shell.side_effect = self._make_shell_side_effect( | ||
| [ | ||
| b"", # md5sum | ||
| b"OK", # directory check | ||
| json.dumps([]).encode(), # first scanfs - success | ||
| # second scan: 3 retry attempts all fail | ||
| SshcpError("an ssh error"), | ||
|
|
@@ -352,6 +362,7 @@ def test_calls_detect_shell_on_first_scan(self): | |
| self.mock_ssh.shell.side_effect = self._make_shell_side_effect( | ||
| [ | ||
| b"", # md5sum | ||
| b"OK", # directory check | ||
| json.dumps([]).encode(), # scanfs | ||
| json.dumps([]).encode(), # second scan: scanfs | ||
| ] | ||
|
|
@@ -372,6 +383,7 @@ def test_raises_nonrecoverable_error_on_failed_scan(self): | |
| self.mock_ssh.shell.side_effect = self._make_shell_side_effect( | ||
| [ | ||
| b"", # md5sum | ||
| b"OK", # directory check | ||
| SshcpError("SystemScannerError: something failed"), # scanfs - no retry | ||
| ] | ||
| ) | ||
|
|
@@ -390,6 +402,7 @@ def test_retries_transient_errors_on_first_run(self): | |
| self.mock_ssh.shell.side_effect = self._make_shell_side_effect( | ||
| [ | ||
| b"", # md5sum | ||
| b"OK", # directory check | ||
| SshcpError("Timed out after 180s"), # scanfs attempt 1 - transient | ||
| json.dumps([]).encode(), # scanfs attempt 2 - success | ||
| ] | ||
|
|
@@ -405,6 +418,7 @@ def test_retries_up_to_max_attempts(self): | |
| self.mock_ssh.shell.side_effect = self._make_shell_side_effect( | ||
| [ | ||
| b"", # md5sum | ||
| b"OK", # directory check | ||
| SshcpError("Timed out after 180s"), # attempt 1 | ||
| SshcpError("Timed out after 180s"), # attempt 2 | ||
| SshcpError("Timed out after 180s"), # attempt 3 | ||
|
|
@@ -424,6 +438,7 @@ def test_no_retry_on_non_transient_first_run_error(self): | |
| self.mock_ssh.shell.side_effect = self._make_shell_side_effect( | ||
| [ | ||
| b"", # md5sum | ||
| b"OK", # directory check | ||
| SshcpError("Incorrect password"), # scanfs - non-transient | ||
| ] | ||
| ) | ||
|
|
@@ -460,13 +475,53 @@ def test_raises_recoverable_error_on_transient_md5sum_failure(self): | |
| scanner.scan() | ||
| self.assertTrue(ctx.exception.recoverable) | ||
|
|
||
| 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'" | ||
| ) | ||
|
Comment on lines
+478
to
+501
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 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 The implementation determines quoting strategy based only on whether The test correctly reflects this behavior, but the behavior is a bug: users configuring 🤖 Prompt for AI Agents |
||
|
|
||
| def test_defaults_to_python3(self): | ||
| """When remote_python_path is empty, defaults to 'python3'""" | ||
| scanner = self._make_scanner() | ||
|
|
||
| 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' '/remote/path/to/scan/script' '/remote/path/to/scan'") | ||
|
|
||
| def test_raises_recoverable_error_on_transient_copy_failure(self): | ||
| """Transient SSH errors during scanfs copy are recoverable""" | ||
| scanner = self._make_scanner() | ||
|
|
||
| self.mock_ssh.shell.side_effect = self._make_shell_side_effect( | ||
| [ | ||
| b"", # md5sum - doesn't match, triggers install | ||
| b"OK", # directory check | ||
| ] | ||
| ) | ||
| self.mock_ssh.copy.side_effect = SshcpError("lost connection") | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.