Fix search path None case.#706
Conversation
There was a problem hiding this comment.
Pull request overview
Adjusts central (rclone-based) search to handle a None search path, intended to support Google Drive configurations where a central base path may be absent.
Changes:
- Updates
search_central_via_connection()to acceptsearch_path: Path | None. - Builds an rclone
lsjsontarget string fromsearch_path, defaulting to an empty path whenNone. - Avoids joining
search_path / namewhensearch_pathisNone.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
This PR updates the central-search helper to handle cases where the central search_path is None (notably for Google Drive configurations where central_path may be unset), preventing crashes when listing/searching the remote root.
Changes:
- Allow
search_central_via_connection()to acceptsearch_path: Path | None. - Introduce
final_search_pathto safely build the rclonelsjsoncommand whensearch_pathisNone. - Adjust returned “full path” construction to work when searching from the remote root.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
for more information, see https://pre-commit.ci
|
@JoeZiminski I've opened a new pull request, #707, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@JoeZiminski I've opened a new pull request, #708, to work on those changes. Once the pull request is ready, I'll request review from you. |
|
@copilot can't you make those changes on this PR? it makes no sense for them to be applied on another PR |
|
@JoeZiminski I've opened a new pull request, #709, to work on those changes. Once the pull request is ready, I'll request review from you. |
…earch_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>
…#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>
There was a problem hiding this comment.
Pull request overview
This PR fixes the search_path=None edge case when searching central storage via rclone (notably needed for Google Drive configurations where cfg["central_path"] can be None), and adds an integration test to prevent regressions.
Changes:
- Update
search_central_via_connection()to acceptsearch_path: Path | None, treatNoneas a remote-root search, and improve error-message context for the root case. - Add an explicit
Noneguard insearch_local_filesystem()to fail fast with a clear error. - Add an integration test covering
search_central_via_connection(..., search_path=None, ...)for bothreturn_full_path=True/False.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tests/tests_integration/test_search_methods.py | Adds integration coverage for search_path=None behavior in central search. |
| datashuttle/utils/folders.py | Implements None handling for central search paths and improves diagnostics; adds a guard for local search. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
central_pathcan beNoneifconnection_methodisgdrive. This case was not handled by validation whensearch_centralisTrue, leading to an error. TheNonecase is explicitly handled here.