Skip to content

Improve rclone transfer output detail. #623

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.

Sign up for GitHub

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

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open

Improve rclone transfer output detail. #623

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
447b2a1
Improve rclone transfer output detail.
JoeZiminski Oct 20, 2025
9f642d7
Full draft implementation for TUI and API.
JoeZiminski Oct 21, 2025
224e3bb
Some tidying up.
JoeZiminski Oct 21, 2025
5cb61be
Extend to critical errors.
JoeZiminski Oct 22, 2025
a81db02
Add test for `errors` output.
JoeZiminski Oct 22, 2025
dd0aeaa
Finalise tests.
JoeZiminski Oct 22, 2025
15d8102
Also show message if nothing was transferred.
JoeZiminski Oct 23, 2025
85f90f6
Docstrings and linting.
JoeZiminski Oct 23, 2025
5afba61
Tidy up tests, add some docstrings.
JoeZiminski Oct 23, 2025
9763df8
Add a bit more documentation.
JoeZiminski Oct 23, 2025
17a51aa
Tidy up and fix tests.
JoeZiminski Oct 24, 2025
7f73b3f
Fix tests...again.
JoeZiminski Oct 24, 2025
4a562a8
Fix tests on linux.
JoeZiminski Oct 24, 2025
55ef242
Playing around windows only...
JoeZiminski Oct 24, 2025
e04c8ae
Tests, different cases for locking file on windows vs. macos/linux.
JoeZiminski Oct 27, 2025
6772736
Minor reformating on error messages.
JoeZiminski Oct 29, 2025
e2de477
Revert worlfkow changes.
JoeZiminski Oct 27, 2025
3153bff
Adjust tests to investigate failures.
JoeZiminski Nov 3, 2025
47a8a90
Fix path separator issues.
JoeZiminski Nov 3, 2025
826534d
Fix newline.
JoeZiminski Nov 3, 2025
bc27a76
Add back all tests.
JoeZiminski Nov 3, 2025
d4834c5
Try again! to fix these weird test failures.
JoeZiminski Nov 3, 2025
06c77c8
Try moving to logging module to take advantage of the logging test ma…
JoeZiminski Nov 5, 2025
ad76736
Fix CI script.
JoeZiminski Nov 5, 2025
5778043
Revert ci changes.
JoeZiminski Nov 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 86 additions & 31 deletions datashuttle/datashuttle_class.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
OverwriteExistingFiles,
Prefix,
TopLevelFolder,
TransferErrors,
)

import yaml
Expand Down Expand Up @@ -315,7 +316,8 @@ def upload_custom(
overwrite_existing_files: OverwriteExistingFiles = "never",
dry_run: bool = False,
init_log: bool = True,
) -> None:
display_errors: bool = True,
) -> TransferErrors:
"""Upload data from a local project to the central project folder.

Parameters
Expand Down Expand Up @@ -354,13 +356,16 @@ def upload_custom(
always be ``True``, unless logger is handled elsewhere
(e.g. in a calling function).

display_errors
if `True`, a summary of errors will be printed alongside the Rclone logs.

"""
if init_log:
self._start_log("upload-custom", local_vars=locals())

self._check_top_level_folder(top_level_folder)

TransferData(
errors = TransferData(
self.cfg,
"upload",
top_level_folder,
Expand All @@ -369,12 +374,16 @@ def upload_custom(
datatype,
overwrite_existing_files,
dry_run,
log=True,
)
).run()

if display_errors:
rclone.log_rclone_copy_errors_api(errors)

if init_log:
ds_logger.close_log_filehandler()

return errors

@check_configs_set
@check_is_not_local_project
def download_custom(
Expand All @@ -386,7 +395,8 @@ def download_custom(
overwrite_existing_files: OverwriteExistingFiles = "never",
dry_run: bool = False,
init_log: bool = True,
) -> None:
display_errors: bool = True,
) -> TransferErrors:
"""Download data from the central project to the local project folder.

Parameters
Expand Down Expand Up @@ -425,13 +435,16 @@ def download_custom(
always be ``True``, unless logger is handled elsewhere
(e.g. in a calling function).

display_errors
if `True`, a summary of errors will be printed alongside the Rclone logs.

"""
if init_log:
self._start_log("download-custom", local_vars=locals())

self._check_top_level_folder(top_level_folder)

TransferData(
errors = TransferData(
self.cfg,
"download",
top_level_folder,
Expand All @@ -440,12 +453,16 @@ def download_custom(
datatype,
overwrite_existing_files,
dry_run,
log=True,
)
).run()

if display_errors:
rclone.log_rclone_copy_errors_api(errors)

if init_log:
ds_logger.close_log_filehandler()

return errors

# Specific top-level folder
# ----------------------------------------------------------------------------------
# A set of convenience functions are provided to abstract
Expand All @@ -457,7 +474,7 @@ def upload_rawdata(
self,
overwrite_existing_files: OverwriteExistingFiles = "never",
dry_run: bool = False,
) -> None:
) -> TransferErrors:
"""Upload all files in the `rawdata` top level folder.

Parameters
Expand All @@ -474,7 +491,7 @@ def upload_rawdata(
transfer was taking place, but no files will be moved.

"""
self._transfer_top_level_folder(
return self._transfer_top_level_folder(
"upload",
"rawdata",
overwrite_existing_files=overwrite_existing_files,
Expand All @@ -487,7 +504,7 @@ def upload_derivatives(
self,
overwrite_existing_files: OverwriteExistingFiles = "never",
dry_run: bool = False,
) -> None:
) -> TransferErrors:
"""Upload all files in the `derivatives` top level folder.

Parameters
Expand All @@ -504,7 +521,7 @@ def upload_derivatives(
transfer was taking place, but no files will be moved.

"""
self._transfer_top_level_folder(
return self._transfer_top_level_folder(
"upload",
"derivatives",
overwrite_existing_files=overwrite_existing_files,
Expand All @@ -517,7 +534,7 @@ def download_rawdata(
self,
overwrite_existing_files: OverwriteExistingFiles = "never",
dry_run: bool = False,
) -> None:
) -> TransferErrors:
"""Download all files in the `rawdata` top level folder.

Parameters
Expand All @@ -534,7 +551,7 @@ def download_rawdata(
transfer was taking place, but no files will be moved..

"""
self._transfer_top_level_folder(
return self._transfer_top_level_folder(
"download",
"rawdata",
overwrite_existing_files=overwrite_existing_files,
Expand All @@ -547,7 +564,7 @@ def download_derivatives(
self,
overwrite_existing_files: OverwriteExistingFiles = "never",
dry_run: bool = False,
) -> None:
) -> TransferErrors:
"""Download all files in the `derivatives` top level folder.

Parameters
Expand All @@ -564,7 +581,7 @@ def download_derivatives(
transfer was taking place, but no files will be moved.

"""
self._transfer_top_level_folder(
return self._transfer_top_level_folder(
"download",
"derivatives",
overwrite_existing_files=overwrite_existing_files,
Expand All @@ -577,7 +594,7 @@ def upload_entire_project(
self,
overwrite_existing_files: OverwriteExistingFiles = "never",
dry_run: bool = False,
) -> None:
) -> TransferErrors:
"""Upload the entire project.

Includes every top level folder (e.g. ``rawdata``, ``derivatives``).
Expand All @@ -597,18 +614,21 @@ def upload_entire_project(

"""
self._start_log("upload-entire-project", local_vars=locals())
self._transfer_entire_project(

errors = self._transfer_entire_project(
"upload", overwrite_existing_files, dry_run
)
ds_logger.close_log_filehandler()

return errors

@check_configs_set
@check_is_not_local_project
def download_entire_project(
self,
overwrite_existing_files: OverwriteExistingFiles = "never",
dry_run: bool = False,
) -> None:
) -> TransferErrors:
"""Download the entire project.

Includes every top level folder (e.g. ``rawdata``, ``derivatives``).
Expand All @@ -628,19 +648,23 @@ def download_entire_project(

"""
self._start_log("download-entire-project", local_vars=locals())
self._transfer_entire_project(

errors = self._transfer_entire_project(
"download", overwrite_existing_files, dry_run
)

ds_logger.close_log_filehandler()

return errors

@check_configs_set
@check_is_not_local_project
def upload_specific_folder_or_file(
self,
filepath: Union[str, Path],
overwrite_existing_files: OverwriteExistingFiles = "never",
dry_run: bool = False,
) -> None:
) -> TransferErrors:
"""Upload a specific file or folder.

If transferring a single file, the path including the filename is
Expand All @@ -666,20 +690,22 @@ def upload_specific_folder_or_file(
"""
self._start_log("upload-specific-folder-or-file", local_vars=locals())

self._transfer_specific_file_or_folder(
errors = self._transfer_specific_file_or_folder(
"upload", filepath, overwrite_existing_files, dry_run
)

ds_logger.close_log_filehandler()

return errors

@check_configs_set
@check_is_not_local_project
def download_specific_folder_or_file(
self,
filepath: Union[str, Path],
overwrite_existing_files: OverwriteExistingFiles = "never",
dry_run: bool = False,
) -> None:
) -> TransferErrors:
"""Download a specific file or folder.

If transferring a single file, the path including the filename is
Expand Down Expand Up @@ -708,20 +734,23 @@ def download_specific_folder_or_file(
"download-specific-folder-or-file", local_vars=locals()
)

self._transfer_specific_file_or_folder(
errors = self._transfer_specific_file_or_folder(
"download", filepath, overwrite_existing_files, dry_run
)

ds_logger.close_log_filehandler()

return errors

def _transfer_top_level_folder(
self,
upload_or_download: Literal["upload", "download"],
top_level_folder: TopLevelFolder,
overwrite_existing_files: OverwriteExistingFiles = "never",
dry_run: bool = False,
init_log: bool = True,
) -> None:
display_errors: bool = True,
) -> TransferErrors:
"""Upload or download files within a particular top-level-folder.

A centralised function to upload or download data within
Expand All @@ -738,22 +767,25 @@ def _transfer_top_level_folder(
else self.download_custom
)

transfer_func(
errors = transfer_func(
top_level_folder,
"all",
"all",
"all",
overwrite_existing_files=overwrite_existing_files,
dry_run=dry_run,
init_log=False,
display_errors=display_errors,
)

if init_log:
ds_logger.close_log_filehandler()

return errors

def _transfer_specific_file_or_folder(
self, upload_or_download, filepath, overwrite_existing_files, dry_run
) -> None:
) -> TransferErrors:
"""Core function for upload/download_specific_folder_or_file()."""
if isinstance(filepath, str):
filepath = Path(filepath)
Expand Down Expand Up @@ -784,6 +816,7 @@ def _transfer_specific_file_or_folder(
processed_filepath = filepath

include_list = [f"--include /{processed_filepath.as_posix()}"]

output = rclone.transfer_data(
self.cfg,
upload_or_download,
Expand All @@ -793,8 +826,13 @@ def _transfer_specific_file_or_folder(
overwrite_existing_files, dry_run
),
)
stdout, stderr, errors = rclone.parse_rclone_copy_output(
top_level_folder, output
)
rclone.log_stdout_stderr_python_api(stdout, stderr)
rclone.log_rclone_copy_errors_api(errors)

utils.log(output.stderr.decode("utf-8"))
return errors

# -------------------------------------------------------------------------
# SSH
Expand Down Expand Up @@ -1434,23 +1472,40 @@ def _transfer_entire_project(
upload_or_download: Literal["upload", "download"],
overwrite_existing_files: OverwriteExistingFiles,
dry_run: bool,
) -> None:
) -> TransferErrors:
"""Transfer the entire project.

i.e. every 'top level folder' (e.g. 'rawdata', 'derivatives').
See ``upload_custom()`` or ``download_custom()`` for parameters.
"""
all_errors = rclone.get_empty_errors_dict()

for top_level_folder in canonical_folders.get_top_level_folders():
utils.log_and_message(f"Transferring `{top_level_folder}`")
utils.log_and_message(
f"\n\n*************************************\n"
f"Transferring `{top_level_folder}`\n"
f"*************************************\n"
)

self._transfer_top_level_folder(
errors = self._transfer_top_level_folder(
upload_or_download,
top_level_folder,
overwrite_existing_files=overwrite_existing_files,
dry_run=dry_run,
init_log=False,
display_errors=False,
)

all_errors["file_names"] += errors["file_names"]
all_errors["messages"] += errors["messages"]

key = f"nothing_was_transferred_{top_level_folder}"
all_errors[key] = errors[key] # type: ignore

rclone.log_rclone_copy_errors_api(all_errors)

return all_errors

def _start_log(
self,
command_name: str,
Expand Down
7 changes: 2 additions & 5 deletions datashuttle/tui/css/tui_menu.tcss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,6 @@ MessageBox {
align: center middle;
}

#messagebox_top_container {
height: 15;
width: 65;
}

#messagebox_message_container {
align: center middle;
overflow: hidden auto;
Expand All @@ -112,6 +107,8 @@ MessageBox {
height: 3;
}

/* NOTE: `messagebox_top_container` width and height are defined on the `MessageBox` class.

Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unclosed comment block. The comment starting with /* NOTE: on line 110 is not properly closed with */. This should be fixed to prevent potential parsing issues.

Suggested change
*/

Copilot uses AI. Check for mistakes.
/* Light Mode Error Screen */

MessageBox:light > #messagebox_top_container {
Expand Down
Loading
Loading