Skip to content

Commit 05924af

Browse files
JoeZiminskiCopilotpre-commit-ci[bot]Copilot
authored
Fix search path None case. (#706)
* Fix search path `None` case. * Fix issues. * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Improve error diagnostics for `search_central_via_connection` when `search_path` is `None` (#708) * Initial plan * Move display_search_path into if block with rclone remote name for None case Co-authored-by: JoeZiminski <55797454+JoeZiminski@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: JoeZiminski <55797454+JoeZiminski@users.noreply.github.com> * Add tests for `search_central_via_connection` with `search_path=None` (#707) * Initial plan * Add test for search_central_via_connection with search_path=None Co-authored-by: JoeZiminski <55797454+JoeZiminski@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: JoeZiminski <55797454+JoeZiminski@users.noreply.github.com> * Revert AI nonsense. * Small tidy up. --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
1 parent eca22ae commit 05924af

2 files changed

Lines changed: 87 additions & 6 deletions

File tree

datashuttle/utils/folders.py

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -608,6 +608,8 @@ def search_for_folders(
608608
local_or_central == "local"
609609
or cfg["connection_method"] == "local_filesystem"
610610
):
611+
assert search_path is not None
612+
611613
if not search_path.exists():
612614
if verbose:
613615
utils.log_and_message(
@@ -631,7 +633,9 @@ def search_for_folders(
631633

632634

633635
def search_local_filesystem(
634-
search_path: Path, search_prefix: str, return_full_path: bool = False
636+
search_path: Path,
637+
search_prefix: str,
638+
return_full_path: bool = False,
635639
) -> tuple[List[Any], List[Any]]:
636640
"""Search local filesystem recursively.
637641
@@ -669,7 +673,7 @@ def search_local_filesystem(
669673

670674
def search_central_via_connection(
671675
cfg: Configs,
672-
search_path: Path,
676+
search_path: Path | None,
673677
search_prefix: str,
674678
return_full_path: bool = False,
675679
) -> tuple[List[Any], List[Any]]:
@@ -686,7 +690,7 @@ def search_central_via_connection(
686690
The path to search (relative to the local or remote drive). For example,
687691
for "local_filesystem" this is the path on the local machine. For any other
688692
connection to central, this is the path on the central storage that has been
689-
connected to.
693+
connected to. Can be `None` if connection method is `gdrive`.
690694
691695
search_prefix
692696
The search string e.g. "sub-*".
@@ -699,18 +703,25 @@ def search_central_via_connection(
699703
cfg["connection_method"]
700704
)
701705

706+
final_search_path = search_path.as_posix() if search_path else ""
707+
702708
output = rclone.call_rclone_for_central_connection(
703709
cfg,
704-
f'lsjson {rclone_config_name}:"{search_path.as_posix()}" {rclone.get_config_arg(cfg)}',
710+
f'lsjson {rclone_config_name}:"{final_search_path}" {rclone.get_config_arg(cfg)}',
705711
pipe_std=True,
706712
)
707713

708714
all_folder_names: list = []
709715
all_filenames: list = []
710716

711717
if output.returncode != 0:
718+
display_search_path = (
719+
f"{rclone_config_name}:<root>"
720+
if search_path is None
721+
else final_search_path
722+
)
712723
utils.log_and_message(
713-
f"Error searching files at {search_path.as_posix()}\n"
724+
f"Error searching files at {display_search_path}\n"
714725
f"{output.stderr.decode('utf-8') if output.stderr else ''}"
715726
)
716727
return all_folder_names, all_filenames
@@ -725,7 +736,9 @@ def search_central_via_connection(
725736

726737
is_dir = file_or_folder.get("IsDir", False)
727738

728-
to_append = search_path / name if return_full_path else name
739+
to_append = (
740+
Path(final_search_path) / name if return_full_path else name
741+
)
729742

730743
if is_dir:
731744
all_folder_names.append(to_append)

tests/tests_integration/test_search_methods.py

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import json
2+
import subprocess
13
from pathlib import Path
24

35
import pytest
@@ -106,3 +108,69 @@ def mock_rclone_caller(_, func, optional=None):
106108
assert central_method_files == local_method_files, (
107109
f"Failed files, search_str: {search_str}, search_path: {search_path}"
108110
)
111+
112+
@pytest.mark.parametrize("return_full_path", [True, False])
113+
def test_search_central_none_search_path(
114+
self, project, monkeypatch, return_full_path
115+
):
116+
"""
117+
Test `search_central_via_connection` when `search_path=None`.
118+
119+
When `search_path` is `None` (as is the case for Google Drive, where
120+
`cfg["central_path"]` can be `None`), the function should search from
121+
the root of the remote drive without error.
122+
123+
Verify that:
124+
- the function handles ``search_path=None`` without raising
125+
- when ``return_full_path=True``, returned items are :class:`~pathlib.Path`
126+
objects relative to the remote root (e.g. ``Path("sub-001")``)
127+
- when ``return_full_path=False``, returned items are plain strings
128+
"""
129+
from datashuttle.utils import rclone
130+
131+
fake_entries = [
132+
{"Name": "sub-001", "IsDir": True},
133+
{"Name": "sub-002", "IsDir": True},
134+
{"Name": "rawdata.md", "IsDir": False},
135+
]
136+
fake_output = subprocess.CompletedProcess(
137+
args=[],
138+
returncode=0,
139+
stdout=json.dumps(fake_entries).encode(),
140+
stderr=b"",
141+
)
142+
143+
captured_commands = []
144+
145+
def mock_call_rclone(cfg, command, pipe_std=False):
146+
captured_commands.append(command)
147+
return fake_output
148+
149+
monkeypatch.setattr(
150+
rclone,
151+
"call_rclone_for_central_connection",
152+
mock_call_rclone,
153+
)
154+
155+
folders, files = search_central_via_connection(
156+
project.cfg,
157+
None,
158+
"*",
159+
return_full_path=return_full_path,
160+
)
161+
162+
assert len(folders) == 2
163+
assert len(files) == 1
164+
165+
if return_full_path:
166+
assert all(isinstance(f, Path) for f in folders)
167+
assert all(isinstance(f, Path) for f in files)
168+
assert sorted(folders) == [Path("sub-001"), Path("sub-002")]
169+
assert files == [Path("rawdata.md")]
170+
else:
171+
assert all(isinstance(f, str) for f in folders)
172+
assert all(isinstance(f, str) for f in files)
173+
assert sorted(folders) == ["sub-001", "sub-002"]
174+
assert files == ["rawdata.md"]
175+
176+
assert len(captured_commands) == 1, "Expected exactly one rclone call"

0 commit comments

Comments
 (0)