From 447b2a1ba352d7106b9a39cad6987a9faedbfa09 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Mon, 20 Oct 2025 18:24:53 +0100 Subject: [PATCH 01/25] Improve rclone transfer output detail. --- datashuttle/datashuttle_class.py | 46 ++++++++++++---------- datashuttle/utils/data_transfer.py | 63 +++++++++++++++++++++++++----- datashuttle/utils/rclone.py | 4 +- 3 files changed, 81 insertions(+), 32 deletions(-) diff --git a/datashuttle/datashuttle_class.py b/datashuttle/datashuttle_class.py index 267990c82..c3e675381 100644 --- a/datashuttle/datashuttle_class.py +++ b/datashuttle/datashuttle_class.py @@ -315,7 +315,7 @@ def upload_custom( overwrite_existing_files: OverwriteExistingFiles = "never", dry_run: bool = False, init_log: bool = True, - ) -> None: + ) -> list[str]: """Upload data from a local project to the central project folder. Parameters @@ -360,7 +360,7 @@ def upload_custom( self._check_top_level_folder(top_level_folder) - TransferData( + errors = TransferData( self.cfg, "upload", top_level_folder, @@ -369,12 +369,13 @@ def upload_custom( datatype, overwrite_existing_files, dry_run, - log=True, - ) + ).run() if init_log: ds_logger.close_log_filehandler() + return errors + @check_configs_set @check_is_not_local_project def download_custom( @@ -386,7 +387,7 @@ def download_custom( overwrite_existing_files: OverwriteExistingFiles = "never", dry_run: bool = False, init_log: bool = True, - ) -> None: + ) -> list[str]: """Download data from the central project to the local project folder. Parameters @@ -431,7 +432,7 @@ def download_custom( self._check_top_level_folder(top_level_folder) - TransferData( + errors = TransferData( self.cfg, "download", top_level_folder, @@ -440,12 +441,13 @@ def download_custom( datatype, overwrite_existing_files, dry_run, - log=True, - ) + ).run() if init_log: ds_logger.close_log_filehandler() + return errors + # Specific top-level folder # ---------------------------------------------------------------------------------- # A set of convenience functions are provided to abstract @@ -457,7 +459,7 @@ def upload_rawdata( self, overwrite_existing_files: OverwriteExistingFiles = "never", dry_run: bool = False, - ) -> None: + ) -> list[str]: """Upload all files in the `rawdata` top level folder. Parameters @@ -474,7 +476,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, @@ -487,7 +489,7 @@ def upload_derivatives( self, overwrite_existing_files: OverwriteExistingFiles = "never", dry_run: bool = False, - ) -> None: + ) -> list[str]: """Upload all files in the `derivatives` top level folder. Parameters @@ -504,7 +506,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, @@ -517,7 +519,7 @@ def download_rawdata( self, overwrite_existing_files: OverwriteExistingFiles = "never", dry_run: bool = False, - ) -> None: + ) -> list[str]: """Download all files in the `rawdata` top level folder. Parameters @@ -534,7 +536,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, @@ -547,7 +549,7 @@ def download_derivatives( self, overwrite_existing_files: OverwriteExistingFiles = "never", dry_run: bool = False, - ) -> None: + ) -> list[str]: """Download all files in the `derivatives` top level folder. Parameters @@ -564,7 +566,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, @@ -597,10 +599,11 @@ 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 @@ -721,7 +724,7 @@ def _transfer_top_level_folder( overwrite_existing_files: OverwriteExistingFiles = "never", dry_run: bool = False, init_log: bool = True, - ) -> None: + ) -> list[str]: """Upload or download files within a particular top-level-folder. A centralised function to upload or download data within @@ -738,7 +741,7 @@ def _transfer_top_level_folder( else self.download_custom ) - transfer_func( + errors = transfer_func( top_level_folder, "all", "all", @@ -751,6 +754,8 @@ def _transfer_top_level_folder( 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: @@ -784,7 +789,8 @@ def _transfer_specific_file_or_folder( processed_filepath = filepath include_list = [f"--include /{processed_filepath.as_posix()}"] - output = rclone.transfer_data( + + output = rclone.transfer_data( # TODO: fix thisd self.cfg, upload_or_download, top_level_folder, diff --git a/datashuttle/utils/data_transfer.py b/datashuttle/utils/data_transfer.py index c21d39bda..151016917 100644 --- a/datashuttle/utils/data_transfer.py +++ b/datashuttle/utils/data_transfer.py @@ -1,3 +1,4 @@ +import json from pathlib import Path from typing import List, Literal, Optional, Union @@ -11,6 +12,37 @@ ) +def parse_rclone_copy_output( + stream: bytes, capture_errors: bool = False +) -> tuple[str, list[str]]: + """""" + split_stream = stream.decode("utf-8").split("\n") + + errors = [] + for idx, line in enumerate(split_stream): + try: + line_json = json.loads(line) + except json.JSONDecodeError: + continue + + if capture_errors: + if line_json["level"] == "error": + if "object" in line_json: + errors.append( + f"The file {line_json['object']} failed to transfer. Reason: {line_json['msg']}" + ) + else: + errors.append(f"ERROR : {line_json['msg']}") + + split_stream[idx] = ( + f"{line_json['time'][:19]} {line_json['level'].upper()} : {line_json['msg']}" + ) + + format_stream = "\n".join(split_stream) + + return format_stream, errors + + class TransferData: """Class to perform data transfers. @@ -33,7 +65,6 @@ def __init__( datatype: Union[str, List[str]], overwrite_existing_files: OverwriteExistingFiles, dry_run: bool, - log: bool, ): """Initialise TransferData. @@ -71,9 +102,6 @@ def __init__( Perform a dry-run of transfer. This will output as if file transfer was taking place, but no files will be moved. - log - if `True`, log and print the transfer output. - """ self.__cfg = cfg self.__upload_or_download = upload_or_download @@ -84,6 +112,8 @@ def __init__( self.__base_folder = self.__cfg.get_base_folder( self.__local_or_central, self.__top_level_folder ) + self.__overwrite_existing_files = overwrite_existing_files + self.__dry_run = dry_run self.sub_names = self.to_list(sub_names) self.ses_names = self.to_list(ses_names) @@ -91,6 +121,7 @@ def __init__( self.check_input_arguments() + def run(self): include_list = self.build_a_list_of_all_files_and_folders_to_transfer() if any(include_list): @@ -99,16 +130,28 @@ def __init__( self.__upload_or_download, self.__top_level_folder, include_list, - cfg.make_rclone_transfer_options( - overwrite_existing_files, dry_run + self.__cfg.make_rclone_transfer_options( + self.__overwrite_existing_files, self.__dry_run ), ) - if log: - utils.log_and_message(output.stderr.decode("utf-8")) + stdout, errors = parse_rclone_copy_output( + output.stdout, capture_errors=True + ) + stderr, errors = parse_rclone_copy_output(output.stderr) + + utils.log_and_message( + f"\n\n************** STDOUT **************\n" + f"{stdout}" + f"\n\n************** STDERR **************\n" + f"{stderr}" + ) + else: - if log: - utils.log_and_message("No files included. None transferred.") + utils.log_and_message("No files included. None transferred.") + errors = [] + + return errors # ------------------------------------------------------------------------- # Build the --include list diff --git a/datashuttle/utils/rclone.py b/datashuttle/utils/rclone.py index fe1909119..8c9d0981c 100644 --- a/datashuttle/utils/rclone.py +++ b/datashuttle/utils/rclone.py @@ -461,14 +461,14 @@ def transfer_data( output = call_rclone_through_script( f"{rclone_args('copy')} " f'"{local_filepath}" "{cfg.get_rclone_config_name()}:' - f'{central_filepath}" {extra_arguments}', + f'{central_filepath}" {extra_arguments} --use-json-log', ) elif upload_or_download == "download": output = call_rclone_through_script( f"{rclone_args('copy')} " f'"{cfg.get_rclone_config_name()}:' - f'{central_filepath}" "{local_filepath}" {extra_arguments}', + f'{central_filepath}" "{local_filepath}" {extra_arguments} --use-json-log', ) return output From 9f642d7910eb4cb3159d35961116326c84d9d494 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Tue, 21 Oct 2025 15:10:16 +0100 Subject: [PATCH 02/25] Full draft implementation for TUI and API. --- datashuttle/datashuttle_class.py | 77 ++++++++++++++++------ datashuttle/tui/css/tui_menu.tcss | 7 +- datashuttle/tui/interface.py | 12 ++-- datashuttle/tui/screens/modal_dialogs.py | 70 +++++++++++++++----- datashuttle/utils/custom_types.py | 2 + datashuttle/utils/data_transfer.py | 44 +------------ datashuttle/utils/rclone.py | 84 +++++++++++++++++++++++- 7 files changed, 205 insertions(+), 91 deletions(-) diff --git a/datashuttle/datashuttle_class.py b/datashuttle/datashuttle_class.py index c3e675381..9bf48a375 100644 --- a/datashuttle/datashuttle_class.py +++ b/datashuttle/datashuttle_class.py @@ -26,6 +26,7 @@ OverwriteExistingFiles, Prefix, TopLevelFolder, + TransferErrors, ) import yaml @@ -315,7 +316,7 @@ def upload_custom( overwrite_existing_files: OverwriteExistingFiles = "never", dry_run: bool = False, init_log: bool = True, - ) -> list[str]: + ) -> TransferErrors: """Upload data from a local project to the central project folder. Parameters @@ -387,7 +388,7 @@ def download_custom( overwrite_existing_files: OverwriteExistingFiles = "never", dry_run: bool = False, init_log: bool = True, - ) -> list[str]: + ) -> TransferErrors: """Download data from the central project to the local project folder. Parameters @@ -459,7 +460,7 @@ def upload_rawdata( self, overwrite_existing_files: OverwriteExistingFiles = "never", dry_run: bool = False, - ) -> list[str]: + ) -> TransferErrors: """Upload all files in the `rawdata` top level folder. Parameters @@ -489,7 +490,7 @@ def upload_derivatives( self, overwrite_existing_files: OverwriteExistingFiles = "never", dry_run: bool = False, - ) -> list[str]: + ) -> TransferErrors: """Upload all files in the `derivatives` top level folder. Parameters @@ -519,7 +520,7 @@ def download_rawdata( self, overwrite_existing_files: OverwriteExistingFiles = "never", dry_run: bool = False, - ) -> list[str]: + ) -> TransferErrors: """Download all files in the `rawdata` top level folder. Parameters @@ -549,7 +550,7 @@ def download_derivatives( self, overwrite_existing_files: OverwriteExistingFiles = "never", dry_run: bool = False, - ) -> list[str]: + ) -> TransferErrors: """Download all files in the `derivatives` top level folder. Parameters @@ -579,7 +580,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``). @@ -599,10 +600,12 @@ def upload_entire_project( """ self._start_log("upload-entire-project", local_vars=locals()) + errors = self._transfer_entire_project( "upload", overwrite_existing_files, dry_run ) ds_logger.close_log_filehandler() + return errors @check_configs_set @@ -611,7 +614,7 @@ 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``). @@ -631,11 +634,15 @@ 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( @@ -643,7 +650,7 @@ def upload_specific_folder_or_file( 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 @@ -669,12 +676,14 @@ 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( @@ -682,7 +691,7 @@ def download_specific_folder_or_file( 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 @@ -711,12 +720,14 @@ 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"], @@ -724,7 +735,8 @@ def _transfer_top_level_folder( overwrite_existing_files: OverwriteExistingFiles = "never", dry_run: bool = False, init_log: bool = True, - ) -> list[str]: + display_errors: bool = True, + ) -> TransferErrors: """Upload or download files within a particular top-level-folder. A centralised function to upload or download data within @@ -751,6 +763,9 @@ def _transfer_top_level_folder( init_log=False, ) + if display_errors: + rclone.log_rclone_copy_errors_api(errors) + if init_log: ds_logger.close_log_filehandler() @@ -758,7 +773,7 @@ def _transfer_top_level_folder( 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) @@ -790,7 +805,7 @@ def _transfer_specific_file_or_folder( include_list = [f"--include /{processed_filepath.as_posix()}"] - output = rclone.transfer_data( # TODO: fix thisd + output = rclone.transfer_data( self.cfg, upload_or_download, top_level_folder, @@ -799,8 +814,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_rclone_output_python_api(stdout, stderr) + rclone.log_rclone_copy_errors_api(errors) - utils.log(output.stderr.decode("utf-8")) + return errors # ------------------------------------------------------------------------- # SSH @@ -1440,23 +1460,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: TransferErrors = { + "file_names": [], + "messages": [], + } + 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"] + + rclone.log_rclone_copy_errors_api(all_errors) + + return all_errors + def _start_log( self, command_name: str, diff --git a/datashuttle/tui/css/tui_menu.tcss b/datashuttle/tui/css/tui_menu.tcss index b3f8b915b..b795d355d 100644 --- a/datashuttle/tui/css/tui_menu.tcss +++ b/datashuttle/tui/css/tui_menu.tcss @@ -91,11 +91,6 @@ MessageBox { align: center middle; } -#messagebox_top_container { - height: 15; - width: 65; - } - #messagebox_message_container { align: center middle; overflow: hidden auto; @@ -112,6 +107,8 @@ MessageBox { height: 3; } +/* NOTE: `messagebox_top_container` width and height are defined on the `MessageBox` class. + /* Light Mode Error Screen */ MessageBox:light > #messagebox_top_container { diff --git a/datashuttle/tui/interface.py b/datashuttle/tui/interface.py index 00f6e0697..0c813ca24 100644 --- a/datashuttle/tui/interface.py +++ b/datashuttle/tui/interface.py @@ -259,14 +259,14 @@ def transfer_entire_project(self, upload: bool) -> InterfaceOutput: else: transfer_func = self.project.download_entire_project - transfer_func( + errors = transfer_func( overwrite_existing_files=self.tui_settings[ "overwrite_existing_files" ], dry_run=self.tui_settings["dry_run"], ) - return True, None + return True, errors except BaseException as e: return False, str(e) @@ -302,14 +302,14 @@ def transfer_top_level_only( else self.project.download_derivatives ) - transfer_func( + errors = transfer_func( overwrite_existing_files=self.tui_settings[ "overwrite_existing_files" ], dry_run=self.tui_settings["dry_run"], ) - return True, None + return True, errors except BaseException as e: return False, str(e) @@ -349,7 +349,7 @@ def transfer_custom_selection( else: transfer_func = self.project.download_custom - transfer_func( + errors = transfer_func( selected_top_level_folder, sub_names=sub_names, ses_names=ses_names, @@ -360,7 +360,7 @@ def transfer_custom_selection( dry_run=self.tui_settings["dry_run"], ) - return True, None + return True, errors except BaseException as e: return False, str(e) diff --git a/datashuttle/tui/screens/modal_dialogs.py b/datashuttle/tui/screens/modal_dialogs.py index 0d4c94c57..d8e676e06 100644 --- a/datashuttle/tui/screens/modal_dialogs.py +++ b/datashuttle/tui/screens/modal_dialogs.py @@ -49,7 +49,13 @@ class MessageBox(ModalScreen): """ - def __init__(self, message: str, border_color: str) -> None: + def __init__( + self, + message: str, + border_color: str, + width: str = "65", + height: str = "15", + ) -> None: """Initialise the MessageBox. Parameters @@ -65,10 +71,12 @@ def __init__(self, message: str, border_color: str) -> None: self.message = message self.border_color = border_color + self.top_container_width = width + self.top_container_height = height def compose(self) -> ComposeResult: """Add widgets to the MessageBox.""" - yield Container( + self.top_container = Container( Container( Static(self.message, id="messagebox_message_label"), id="messagebox_message_container", @@ -77,6 +85,8 @@ def compose(self) -> ComposeResult: id="messagebox_top_container", ) + yield self.top_container + def on_mount(self) -> None: """Update widgets immediately after mounting.""" if self.border_color == "red": @@ -93,6 +103,9 @@ def on_mount(self) -> None: color, ) + self.top_container.styles.width = self.top_container_width + self.top_container.styles.height = self.top_container_height + def on_button_pressed(self) -> None: """Handle button press.""" self.dismiss(True) @@ -219,23 +232,44 @@ def on_button_pressed(self, event: Button.Pressed) -> None: async def handle_transfer_and_update_ui_when_complete(self) -> None: """Run the data transfer worker and updates the UI on completion.""" - data_transfer_worker = self.transfer_func() - await data_transfer_worker.wait() - success, output = data_transfer_worker.result - self.dismiss() - - if success: - self.app.push_screen( - MessageBox( - "Transfer finished." - "\n\n" - "Check the most recent logs to " - "ensure transfer completed successfully.", - border_color="grey", + try: + data_transfer_worker = self.transfer_func() + await data_transfer_worker.wait() + success, output = data_transfer_worker.result + + self.dismiss() + + if success: + errors = output + + if any(errors["messages"]): + errors_message = "[red]Errors detected, in files:[/red]\n" + errors_message += "\n".join(errors["file_names"]) + errors_message += ( + "[red]\n\nThe error messages are:[/red]\n" + ) + errors_message += "\n\n".join(errors["messages"]) + messagebox_kwargs = {"width": "75%", "height": "75%"} + else: + errors_message = "No errors detected" + messagebox_kwargs = {} + + message = ( + f"Transfer finished.\n\n" + f"{errors_message}\n\n" + f"Check the most recent logs for full details." ) - ) - else: - self.app.show_modal_error_dialog(output) + + self.app.push_screen( + MessageBox( + message, border_color="grey", **messagebox_kwargs + ) + ) + else: + self.app.show_modal_error_dialog(output) + + except BaseException as e: + self.app.show_modal_error_dialog(str(e)) class SearchingCentralForNextSubSesPopup(ModalScreen): diff --git a/datashuttle/utils/custom_types.py b/datashuttle/utils/custom_types.py index 44c8de353..0ff29dc90 100644 --- a/datashuttle/utils/custom_types.py +++ b/datashuttle/utils/custom_types.py @@ -13,3 +13,5 @@ ConnectionMethods = Literal[ "ssh", "local_filesystem", "gdrive", "aws", "local_only" ] + +TransferErrors = dict[str, list[str]] diff --git a/datashuttle/utils/data_transfer.py b/datashuttle/utils/data_transfer.py index 151016917..6e5435293 100644 --- a/datashuttle/utils/data_transfer.py +++ b/datashuttle/utils/data_transfer.py @@ -1,4 +1,3 @@ -import json from pathlib import Path from typing import List, Literal, Optional, Union @@ -12,37 +11,6 @@ ) -def parse_rclone_copy_output( - stream: bytes, capture_errors: bool = False -) -> tuple[str, list[str]]: - """""" - split_stream = stream.decode("utf-8").split("\n") - - errors = [] - for idx, line in enumerate(split_stream): - try: - line_json = json.loads(line) - except json.JSONDecodeError: - continue - - if capture_errors: - if line_json["level"] == "error": - if "object" in line_json: - errors.append( - f"The file {line_json['object']} failed to transfer. Reason: {line_json['msg']}" - ) - else: - errors.append(f"ERROR : {line_json['msg']}") - - split_stream[idx] = ( - f"{line_json['time'][:19]} {line_json['level'].upper()} : {line_json['msg']}" - ) - - format_stream = "\n".join(split_stream) - - return format_stream, errors - - class TransferData: """Class to perform data transfers. @@ -135,17 +103,11 @@ def run(self): ), ) - stdout, errors = parse_rclone_copy_output( - output.stdout, capture_errors=True + stdout, stderr, errors = rclone.parse_rclone_copy_output( + self.__top_level_folder, output ) - stderr, errors = parse_rclone_copy_output(output.stderr) - utils.log_and_message( - f"\n\n************** STDOUT **************\n" - f"{stdout}" - f"\n\n************** STDERR **************\n" - f"{stderr}" - ) + rclone.log_rclone_output_python_api(stdout, stderr) else: utils.log_and_message("No files included. None transferred.") diff --git a/datashuttle/utils/rclone.py b/datashuttle/utils/rclone.py index 8c9d0981c..71ce66067 100644 --- a/datashuttle/utils/rclone.py +++ b/datashuttle/utils/rclone.py @@ -4,8 +4,9 @@ if TYPE_CHECKING: from datashuttle.configs.config_class import Configs - from datashuttle.utils.custom_types import TopLevelFolder + from datashuttle.utils.custom_types import TopLevelFolder, TransferErrors +import json import os import platform import shlex @@ -474,6 +475,87 @@ def transfer_data( return output +def log_rclone_output_python_api(stdout, stderr): + """""" + message = ( + f"\n\n************** STDOUT **************\n" + f"{stdout}" + f"\n\n************** STDERR **************\n" + f"{stderr}" + ) + + utils.log_and_message(message) + + +def log_rclone_copy_errors_api(errors): + """""" + if any(errors["messages"]): + message = ( + "\n\nErrors were detected, in files:" + "\n-------------------------------\n" + ) + message += "\n".join(errors["file_names"]) + message += "\n\nThe error messages are:\n-----------------------\n" + message += "\n".join(errors["messages"]) + message += "\n" + else: + message = "No transfer errors were detected.\n" + + utils.log_and_message(message) + + +def parse_rclone_copy_output(top_level_folder, output): + """""" + stdout, errors = reformat_rclone_copy_output( + output.stdout, capture_errors=True, top_level_folder=top_level_folder + ) + + stderr, _ = reformat_rclone_copy_output(output.stderr) + + return stdout, stderr, errors + + +def reformat_rclone_copy_output( + stream: bytes, + capture_errors: bool = False, + top_level_folder: str | None = None, +) -> tuple[str, TransferErrors]: + """""" + # TODO: assert top level fodlers + + split_stream = stream.decode("utf-8").split("\n") + + errors: TransferErrors = { + "file_names": [], + "messages": [], + } + + for idx, line in enumerate(split_stream): + try: + line_json = json.loads(line) + except json.JSONDecodeError: + continue + + if capture_errors: + if line_json["level"] == "error": + if "object" in line_json: + full_filepath = f"{top_level_folder}/{line_json['object']}" + errors["file_names"].append(full_filepath) + errors["messages"].append( + f"The file {full_filepath} failed to transfer. Reason: {line_json['msg']}" + ) + else: + errors["messages"].append(f"ERROR : {line_json['msg']}") + + split_stream[idx] = ( + f"{line_json['time'][:19]} {line_json['level'].upper()} : {line_json['msg']}" + ) + + format_stream = "\n".join(split_stream) + + return format_stream, errors + + def get_local_and_central_file_differences( cfg: Configs, top_level_folders_to_check: List[TopLevelFolder], From 224e3bbe477f8de0aa0edca40d4d84a3bd5a8454 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Tue, 21 Oct 2025 16:33:05 +0100 Subject: [PATCH 03/25] Some tidying up. --- datashuttle/tui/interface.py | 2 +- datashuttle/utils/rclone.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/datashuttle/tui/interface.py b/datashuttle/tui/interface.py index 0c813ca24..cc5be7a78 100644 --- a/datashuttle/tui/interface.py +++ b/datashuttle/tui/interface.py @@ -272,7 +272,7 @@ def transfer_entire_project(self, upload: bool) -> InterfaceOutput: return False, str(e) def transfer_top_level_only( - self, selected_top_level_folder: str, upload: bool + self, selected_top_level_folder: TopLevelFolder, upload: bool ) -> InterfaceOutput: """Transfer all files within a selected top level folder. diff --git a/datashuttle/utils/rclone.py b/datashuttle/utils/rclone.py index 71ce66067..4b6ffbe9a 100644 --- a/datashuttle/utils/rclone.py +++ b/datashuttle/utils/rclone.py @@ -518,7 +518,7 @@ def parse_rclone_copy_output(top_level_folder, output): def reformat_rclone_copy_output( stream: bytes, capture_errors: bool = False, - top_level_folder: str | None = None, + top_level_folder: TopLevelFolder | None = None, ) -> tuple[str, TransferErrors]: """""" # TODO: assert top level fodlers From 5cb61bea7f58879fa8b11d1a82cd1d618089c979 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Wed, 22 Oct 2025 15:18:49 +0100 Subject: [PATCH 04/25] Extend to critical errors. --- datashuttle/tui/screens/modal_dialogs.py | 9 +++++-- datashuttle/utils/data_transfer.py | 6 +++++ datashuttle/utils/rclone.py | 32 +++++++++++++++--------- 3 files changed, 33 insertions(+), 14 deletions(-) diff --git a/datashuttle/tui/screens/modal_dialogs.py b/datashuttle/tui/screens/modal_dialogs.py index d8e676e06..64e2fa590 100644 --- a/datashuttle/tui/screens/modal_dialogs.py +++ b/datashuttle/tui/screens/modal_dialogs.py @@ -243,8 +243,13 @@ async def handle_transfer_and_update_ui_when_complete(self) -> None: errors = output if any(errors["messages"]): - errors_message = "[red]Errors detected, in files:[/red]\n" - errors_message += "\n".join(errors["file_names"]) + if errors["file_names"]: + errors_message = ( + "[red]Errors detected! in files:[/red]\n" + ) + errors_message += "\n".join(errors["file_names"]) + else: + errors_message = "[red]Errors detected![/red]" errors_message += ( "[red]\n\nThe error messages are:[/red]\n" ) diff --git a/datashuttle/utils/data_transfer.py b/datashuttle/utils/data_transfer.py index 6e5435293..8eabbfa34 100644 --- a/datashuttle/utils/data_transfer.py +++ b/datashuttle/utils/data_transfer.py @@ -107,6 +107,12 @@ def run(self): self.__top_level_folder, output ) + if output.returncode != 0 and not any(errors["messages"]): + raise RuntimeError( + "Errors were detected in transfer but not reported properly. " + "Please contact the datashuttle team." + ) + rclone.log_rclone_output_python_api(stdout, stderr) else: diff --git a/datashuttle/utils/rclone.py b/datashuttle/utils/rclone.py index 4b6ffbe9a..e66dc90f9 100644 --- a/datashuttle/utils/rclone.py +++ b/datashuttle/utils/rclone.py @@ -490,29 +490,39 @@ def log_rclone_output_python_api(stdout, stderr): def log_rclone_copy_errors_api(errors): """""" if any(errors["messages"]): - message = ( - "\n\nErrors were detected, in files:" - "\n-------------------------------\n" - ) - message += "\n".join(errors["file_names"]) + if any(errors["file_names"]): + message = ( + "\n\nErrors were detected! In files:" + "\n-------------------------------\n" + ) + message += "\n".join(errors["file_names"]) + else: + message = "\n\n[red]Errors detected![/red]" message += "\n\nThe error messages are:\n-----------------------\n" message += "\n".join(errors["messages"]) message += "\n" else: message = "No transfer errors were detected.\n" - utils.log_and_message(message) + utils.log_and_message(message, use_rich=True) def parse_rclone_copy_output(top_level_folder, output): """""" - stdout, errors = reformat_rclone_copy_output( + stdout, out_errors = reformat_rclone_copy_output( output.stdout, capture_errors=True, top_level_folder=top_level_folder ) - stderr, _ = reformat_rclone_copy_output(output.stderr) + stderr, err_errors = reformat_rclone_copy_output( + output.stderr, capture_errors=True + ) - return stdout, stderr, errors + all_errors = { + "file_names": out_errors["file_names"] + err_errors["file_names"], + "messages": out_errors["messages"] + err_errors["messages"], + } + + return stdout, stderr, all_errors def reformat_rclone_copy_output( @@ -521,8 +531,6 @@ def reformat_rclone_copy_output( top_level_folder: TopLevelFolder | None = None, ) -> tuple[str, TransferErrors]: """""" - # TODO: assert top level fodlers - split_stream = stream.decode("utf-8").split("\n") errors: TransferErrors = { @@ -537,7 +545,7 @@ def reformat_rclone_copy_output( continue if capture_errors: - if line_json["level"] == "error": + if line_json["level"] in ["error", "critical"]: if "object" in line_json: full_filepath = f"{top_level_folder}/{line_json['object']}" errors["file_names"].append(full_filepath) From a81db02fe4c0ede70cab0d2422867373875c7c63 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Wed, 22 Oct 2025 15:24:13 +0100 Subject: [PATCH 05/25] Add test for `errors` output. --- .../local_filesystem/test_transfer.py | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/tests/tests_transfers/local_filesystem/test_transfer.py b/tests/tests_transfers/local_filesystem/test_transfer.py index b924650e5..180eb8e21 100644 --- a/tests/tests_transfers/local_filesystem/test_transfer.py +++ b/tests/tests_transfers/local_filesystem/test_transfer.py @@ -641,6 +641,77 @@ def test_overwrite_different_size_different_times( elif overwrite_existing_files == "always": assert test_utils.read_file(central_file_path) == ["file earlier"] + def test_errors_variable(self, project, monkeypatch): + """ + Test that the `errors` variable is correctly + returned from every transfer function. `errors` is a variable + that contains information about any errors that were encountered + during transfer. + """ + + def test_errors(top_level_folder): + return { + "file_names": [f"{top_level_folder}/hello_world.txt"], + "messages": ["how are you?"], + } + + def monkeypatch_parse_output(top_level_folder, b): + stdout = "stdout" + stderr = "stderr" + return stdout, stderr, test_errors(top_level_folder) + + import datashuttle + + monkeypatch.setattr( + datashuttle.utils.rclone, + "parse_rclone_copy_output", + monkeypatch_parse_output, + ) + + subs, sessions = test_utils.get_default_sub_sessions_to_test() + + for top_level_folder in ["rawdata", "derivatives"]: + test_utils.make_and_check_local_project_folders( + project, + top_level_folder, + subs, + sessions, + get_broad_datatypes(), + ) + + specific_file = ( + lambda path_: f"{path_}/rawdata/{subs[0]}/{sessions[0]}/ephys/placeholder_file.txt" + ) + + for func in [ + lambda: project.upload_specific_folder_or_file( + specific_file(project.get_local_path()) + ), + lambda: project.download_specific_folder_or_file( + specific_file(project.get_central_path()) + ), + lambda: project.upload_custom("rawdata", "all", "all", "all"), + lambda: project.download_custom("rawdata", "all", "all", "all"), + project.upload_rawdata, + project.download_rawdata, + ]: + assert func() == test_errors("rawdata") + + for func in [project.upload_derivatives, project.download_derivatives]: + assert func() == test_errors("derivatives") + + for func in [ + project.upload_entire_project, + project.download_entire_project, + ]: + assert func() == { + "file_names": [ + "rawdata/hello_world.txt", + "derivatives/hello_world.txt", + ], + "messages": ["how are you?", "how are you?"], + } + def get_paths_to_a_local_and_central_file(self, project, top_level_folder): path_to_test_file = ( Path(top_level_folder) From dd0aeaaa469293ef195176f8d1021cb5e3d6cda6 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Wed, 22 Oct 2025 17:19:04 +0100 Subject: [PATCH 06/25] Finalise tests. --- datashuttle/datashuttle_class.py | 18 +++++- datashuttle/utils/data_transfer.py | 2 +- datashuttle/utils/rclone.py | 14 +++-- tests/tests_integration/test_logging.py | 2 + .../local_filesystem/test_transfer.py | 43 +++++++++++++ tests/tests_tui/test_tui_transfer.py | 60 ++++++++++++++++++- tests/tests_tui/tui_base.py | 7 ++- 7 files changed, 134 insertions(+), 12 deletions(-) diff --git a/datashuttle/datashuttle_class.py b/datashuttle/datashuttle_class.py index 9bf48a375..01233b72c 100644 --- a/datashuttle/datashuttle_class.py +++ b/datashuttle/datashuttle_class.py @@ -316,6 +316,7 @@ def upload_custom( overwrite_existing_files: OverwriteExistingFiles = "never", dry_run: bool = False, init_log: bool = True, + display_errors: bool = True, ) -> TransferErrors: """Upload data from a local project to the central project folder. @@ -355,6 +356,9 @@ 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()) @@ -372,6 +376,9 @@ def upload_custom( dry_run, ).run() + if display_errors: + rclone.log_rclone_copy_errors_api(errors) + if init_log: ds_logger.close_log_filehandler() @@ -388,6 +395,7 @@ def download_custom( overwrite_existing_files: OverwriteExistingFiles = "never", dry_run: bool = False, init_log: bool = True, + display_errors: bool = True, ) -> TransferErrors: """Download data from the central project to the local project folder. @@ -427,6 +435,9 @@ 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()) @@ -444,6 +455,9 @@ def download_custom( dry_run, ).run() + if display_errors: + rclone.log_rclone_copy_errors_api(errors) + if init_log: ds_logger.close_log_filehandler() @@ -761,11 +775,9 @@ def _transfer_top_level_folder( overwrite_existing_files=overwrite_existing_files, dry_run=dry_run, init_log=False, + display_errors=display_errors, ) - if display_errors: - rclone.log_rclone_copy_errors_api(errors) - if init_log: ds_logger.close_log_filehandler() diff --git a/datashuttle/utils/data_transfer.py b/datashuttle/utils/data_transfer.py index 8eabbfa34..1ff9222fa 100644 --- a/datashuttle/utils/data_transfer.py +++ b/datashuttle/utils/data_transfer.py @@ -117,7 +117,7 @@ def run(self): else: utils.log_and_message("No files included. None transferred.") - errors = [] + errors = rclone.get_empty_errors_dict() return errors diff --git a/datashuttle/utils/rclone.py b/datashuttle/utils/rclone.py index e66dc90f9..23df20f64 100644 --- a/datashuttle/utils/rclone.py +++ b/datashuttle/utils/rclone.py @@ -522,9 +522,18 @@ def parse_rclone_copy_output(top_level_folder, output): "messages": out_errors["messages"] + err_errors["messages"], } + all_errors["file_names"] = list(set(all_errors["file_names"])) + return stdout, stderr, all_errors +def get_empty_errors_dict() -> TransferErrors: + return { + "file_names": [], + "messages": [], + } + + def reformat_rclone_copy_output( stream: bytes, capture_errors: bool = False, @@ -533,10 +542,7 @@ def reformat_rclone_copy_output( """""" split_stream = stream.decode("utf-8").split("\n") - errors: TransferErrors = { - "file_names": [], - "messages": [], - } + errors = get_empty_errors_dict() for idx, line in enumerate(split_stream): try: diff --git a/tests/tests_integration/test_logging.py b/tests/tests_integration/test_logging.py index 9b5bd4a0b..4dac348ab 100644 --- a/tests/tests_integration/test_logging.py +++ b/tests/tests_integration/test_logging.py @@ -291,6 +291,7 @@ def test_logs_upload_and_download( assert "--include" in log assert "sub-11/ses-123/anat/**" in log assert "/central/test_project/rawdata" in log + assert "No transfer errors were detected" in log @pytest.mark.parametrize("upload_or_download", ["upload", "download"]) def test_logs_upload_and_download_folder_or_file( @@ -329,6 +330,7 @@ def test_logs_upload_and_download_folder_or_file( ) assert "sub-001/ses-001" in log assert "Elapsed time" in log + assert "No transfer errors were detected" in log # ---------------------------------------------------------------------------------- # Test temporary logging path diff --git a/tests/tests_transfers/local_filesystem/test_transfer.py b/tests/tests_transfers/local_filesystem/test_transfer.py index 180eb8e21..a6dac479b 100644 --- a/tests/tests_transfers/local_filesystem/test_transfer.py +++ b/tests/tests_transfers/local_filesystem/test_transfer.py @@ -647,6 +647,8 @@ def test_errors_variable(self, project, monkeypatch): returned from every transfer function. `errors` is a variable that contains information about any errors that were encountered during transfer. + + DOCS TODO this doesn't matter if its top level folder or whatever """ def test_errors(top_level_folder): @@ -712,6 +714,47 @@ def monkeypatch_parse_output(top_level_folder, b): "messages": ["how are you?", "how are you?"], } + def test_errors_are_caught_and_logged(self, project): + """""" # TODO DOC OMD TODO! + subs, sessions = test_utils.get_default_sub_sessions_to_test() + + test_utils.make_and_check_local_project_folders( + project, + "rawdata", + subs, + sessions, + get_broad_datatypes(), + ) + + relative_path = ( + Path("rawdata") + / subs[0] + / sessions[0] + / "ephys" + / "placeholder_file.txt" + ) + a_transferred_file = project.get_local_path() / relative_path + + test_utils.delete_log_files(project.cfg.logging_path) + + from filelock import FileLock + + lock = FileLock(a_transferred_file, timeout=5) + with lock: + errors = project.upload_custom("rawdata", "all", "all", "all") + + assert errors["file_names"] == [relative_path.as_posix()] + assert ( + "another process has locked a portion of the file" + in errors["messages"][0] + ) + + log = test_utils.read_log_file(project.cfg.logging_path) + + assert errors["file_names"][0] in log + assert "Errors were detected!" in log + assert "another process has locked a portion of the file" in log + def get_paths_to_a_local_and_central_file(self, project, top_level_folder): path_to_test_file = ( Path(top_level_folder) diff --git a/tests/tests_tui/test_tui_transfer.py b/tests/tests_tui/test_tui_transfer.py index 03b036508..44f2d6f99 100644 --- a/tests/tests_tui/test_tui_transfer.py +++ b/tests/tests_tui/test_tui_transfer.py @@ -218,6 +218,60 @@ async def test_transfer_custom( await pilot.pause() + @pytest.mark.asyncio + async def test_errors_are_reported_on_pop_up(self, setup_project_paths): + """""" + tmp_config_path, tmp_path, project_name = setup_project_paths.values() + + subs, sessions = test_utils.get_default_sub_sessions_to_test() + + app = TuiApp() + async with app.run_test(size=self.tui_size()) as pilot: + await self.check_and_click_onto_existing_project( + pilot, project_name + ) + await self.switch_tab(pilot, "transfer") + + project = pilot.app.screen.interface.project + + self.setup_project_for_data_transfer( + project, + subs, + sessions, + ["rawdata", "derivatives"], + "upload", + ) + + from pathlib import Path + + relative_path = ( + Path("rawdata") + / subs[0] + / sessions[0] + / "ephys" + / "placeholder_file.txt" + ) + a_transferred_file = project.get_local_path() / relative_path + + from filelock import FileLock + + lock = FileLock(a_transferred_file, timeout=5) + with lock: + await self.run_transfer( + pilot, "upload", close_final_messagebox=False + ) + + from datashuttle.tui.screens.modal_dialogs import MessageBox + + assert isinstance(app.screen, MessageBox) + displayed_message = app.screen.message + + assert relative_path.as_posix() in displayed_message + assert ( + " another process has locked a portion of the file" + in displayed_message + ) + async def switch_top_level_folder_select( self, pilot, id, top_level_folder ): @@ -227,14 +281,16 @@ async def switch_top_level_folder_select( await self.move_select_to_position(pilot, id, position=5) assert pilot.app.screen.query_one(id).value == "derivatives" - async def run_transfer(self, pilot, upload_or_download): + async def run_transfer( + self, pilot, upload_or_download, close_final_messagebox=True + ): # Check assumed default is correct on the transfer switch assert pilot.app.screen.query_one("#transfer_switch").value is False if upload_or_download == "download": await self.scroll_to_click_pause(pilot, "#transfer_switch") - await self.click_and_await_transfer(pilot) + await self.click_and_await_transfer(pilot, close_final_messagebox) def setup_project_for_data_transfer( self, diff --git a/tests/tests_tui/tui_base.py b/tests/tests_tui/tui_base.py index a0c8f16d6..2d08e5ffe 100644 --- a/tests/tests_tui/tui_base.py +++ b/tests/tests_tui/tui_base.py @@ -239,7 +239,9 @@ async def move_select_to_position(self, pilot, id, position): await pilot.click(id, offset=(2, position)) await pilot.pause() - async def click_and_await_transfer(self, pilot): + async def click_and_await_transfer( + self, pilot, close_final_messagebox=True + ): await self.scroll_to_click_pause(pilot, "#transfer_transfer_button") await self.scroll_to_click_pause(pilot, "#confirm_ok_button") @@ -248,7 +250,8 @@ async def click_and_await_transfer(self, pilot): if transfer_task: await transfer_task - await self.close_messagebox(pilot) + if close_final_messagebox: + await self.close_messagebox(pilot) async def double_click_input(self, pilot, sub_or_ses, control=False): """Helper function to double click input to suggest next sub or ses. From 15d8102c5c9a4c9041009975eb23794a00ed35b6 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Thu, 23 Oct 2025 15:16:53 +0100 Subject: [PATCH 07/25] Also show message if nothing was transferred. --- datashuttle/datashuttle_class.py | 9 ++-- datashuttle/tui/screens/modal_dialogs.py | 30 +++++++++--- datashuttle/utils/custom_types.py | 23 ++++++---- datashuttle/utils/data_transfer.py | 1 + datashuttle/utils/rclone.py | 58 ++++++++++++++++-------- 5 files changed, 84 insertions(+), 37 deletions(-) diff --git a/datashuttle/datashuttle_class.py b/datashuttle/datashuttle_class.py index 01233b72c..073a3a1cc 100644 --- a/datashuttle/datashuttle_class.py +++ b/datashuttle/datashuttle_class.py @@ -1478,10 +1478,7 @@ def _transfer_entire_project( i.e. every 'top level folder' (e.g. 'rawdata', 'derivatives'). See ``upload_custom()`` or ``download_custom()`` for parameters. """ - all_errors: TransferErrors = { - "file_names": [], - "messages": [], - } + all_errors = rclone.get_empty_errors_dict() for top_level_folder in canonical_folders.get_top_level_folders(): utils.log_and_message( @@ -1502,6 +1499,10 @@ def _transfer_entire_project( 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 diff --git a/datashuttle/tui/screens/modal_dialogs.py b/datashuttle/tui/screens/modal_dialogs.py index 64e2fa590..1e7512042 100644 --- a/datashuttle/tui/screens/modal_dialogs.py +++ b/datashuttle/tui/screens/modal_dialogs.py @@ -242,26 +242,42 @@ async def handle_transfer_and_update_ui_when_complete(self) -> None: if success: errors = output + errors_message = "" + + messagebox_kwargs = {} + + no_transfer_col = ( + "blue" + if self.app.theme == "textual-light" + else "lightblue" + ) + + if errors["nothing_was_transferred_rawdata"] is True: + errors_message += f"[{no_transfer_col}]\nNote! Nothing was transferred from rawdata[/{no_transfer_col}]\n" + + if errors["nothing_was_transferred_derivatives"] is True: + errors_message += f"[{no_transfer_col}]\nNote! Nothing was transferred from derivatives[/{no_transfer_col}]\n" + if any(errors["messages"]): if errors["file_names"]: - errors_message = ( + errors_message += ( "[red]Errors detected! in files:[/red]\n" ) errors_message += "\n".join(errors["file_names"]) else: - errors_message = "[red]Errors detected![/red]" + errors_message += "[red]Errors detected![/red]" errors_message += ( "[red]\n\nThe error messages are:[/red]\n" ) errors_message += "\n\n".join(errors["messages"]) messagebox_kwargs = {"width": "75%", "height": "75%"} - else: - errors_message = "No errors detected" - messagebox_kwargs = {} + + if errors_message == "": + errors_message += "No errors detected" message = ( - f"Transfer finished.\n\n" - f"{errors_message}\n\n" + f"Transfer finished.\n" + f"{errors_message}\n" f"Check the most recent logs for full details." ) diff --git a/datashuttle/utils/custom_types.py b/datashuttle/utils/custom_types.py index 0ff29dc90..f1407d02f 100644 --- a/datashuttle/utils/custom_types.py +++ b/datashuttle/utils/custom_types.py @@ -1,17 +1,24 @@ -from typing import Any, Literal, Tuple +from typing import Any, Literal, Tuple, TypeAlias, TypedDict -DisplayMode = Literal["error", "warn", "print"] +DisplayMode: TypeAlias = Literal["error", "warn", "print"] -TopLevelFolder = Literal["rawdata", "derivatives"] +TopLevelFolder: TypeAlias = Literal["rawdata", "derivatives"] -OverwriteExistingFiles = Literal["never", "always", "if_source_newer"] +OverwriteExistingFiles: TypeAlias = Literal[ + "never", "always", "if_source_newer" +] -Prefix = Literal["sub", "ses"] +Prefix: TypeAlias = Literal["sub", "ses"] -InterfaceOutput = Tuple[bool, Any] +InterfaceOutput: TypeAlias = Tuple[bool, Any] -ConnectionMethods = Literal[ +ConnectionMethods: TypeAlias = Literal[ "ssh", "local_filesystem", "gdrive", "aws", "local_only" ] -TransferErrors = dict[str, list[str]] + +class TransferErrors(TypedDict): + file_names: list[str] + messages: list[str] + nothing_was_transferred_rawdata: bool | None + nothing_was_transferred_derivatives: bool | None diff --git a/datashuttle/utils/data_transfer.py b/datashuttle/utils/data_transfer.py index 1ff9222fa..3b36a55ee 100644 --- a/datashuttle/utils/data_transfer.py +++ b/datashuttle/utils/data_transfer.py @@ -118,6 +118,7 @@ def run(self): else: utils.log_and_message("No files included. None transferred.") errors = rclone.get_empty_errors_dict() + errors[f"nothing_was_transferred_{self.__top_level_folder}"] = True return errors diff --git a/datashuttle/utils/rclone.py b/datashuttle/utils/rclone.py index 23df20f64..5b1ffc653 100644 --- a/datashuttle/utils/rclone.py +++ b/datashuttle/utils/rclone.py @@ -489,19 +489,28 @@ def log_rclone_output_python_api(stdout, stderr): def log_rclone_copy_errors_api(errors): """""" + message = "" + + if errors["nothing_was_transferred_rawdata"] is True: + message += "\n\nNote! Nothing was transferred from rawdata!\n" + + if errors["nothing_was_transferred_derivatives"] is True: + message += "\n\nNote! Nothing was transferred from derivatives!\n" + if any(errors["messages"]): if any(errors["file_names"]): - message = ( + message += ( "\n\nErrors were detected! In files:" "\n-------------------------------\n" ) message += "\n".join(errors["file_names"]) else: - message = "\n\n[red]Errors detected![/red]" + message += "\n\n[red]Errors detected![/red]" message += "\n\nThe error messages are:\n-----------------------\n" message += "\n".join(errors["messages"]) message += "\n" - else: + + if message == "": message = "No transfer errors were detected.\n" utils.log_and_message(message, use_rich=True) @@ -510,16 +519,23 @@ def log_rclone_copy_errors_api(errors): def parse_rclone_copy_output(top_level_folder, output): """""" stdout, out_errors = reformat_rclone_copy_output( - output.stdout, capture_errors=True, top_level_folder=top_level_folder + output.stdout, top_level_folder=top_level_folder ) stderr, err_errors = reformat_rclone_copy_output( - output.stderr, capture_errors=True + output.stderr, top_level_folder=top_level_folder ) all_errors = { - "file_names": out_errors["file_names"] + err_errors["file_names"], + "file_names": out_errors["file_names"] + + err_errors["file_names"], # TODO: this is so messy "messages": out_errors["messages"] + err_errors["messages"], + "nothing_was_transferred_rawdata": err_errors[ + "nothing_was_transferred_rawdata" + ], + "nothing_was_transferred_derivatives": err_errors[ + "nothing_was_transferred_derivatives" + ], } all_errors["file_names"] = list(set(all_errors["file_names"])) @@ -531,15 +547,16 @@ def get_empty_errors_dict() -> TransferErrors: return { "file_names": [], "messages": [], + "nothing_was_transferred_rawdata": None, + "nothing_was_transferred_derivatives": None, } def reformat_rclone_copy_output( stream: bytes, - capture_errors: bool = False, top_level_folder: TopLevelFolder | None = None, ) -> tuple[str, TransferErrors]: - """""" + """:return:""" split_stream = stream.decode("utf-8").split("\n") errors = get_empty_errors_dict() @@ -550,16 +567,21 @@ def reformat_rclone_copy_output( except json.JSONDecodeError: continue - if capture_errors: - if line_json["level"] in ["error", "critical"]: - if "object" in line_json: - full_filepath = f"{top_level_folder}/{line_json['object']}" - errors["file_names"].append(full_filepath) - errors["messages"].append( - f"The file {full_filepath} failed to transfer. Reason: {line_json['msg']}" - ) - else: - errors["messages"].append(f"ERROR : {line_json['msg']}") + if line_json["level"] in ["error", "critical"]: + if "object" in line_json: + full_filepath = f"{top_level_folder}/{line_json['object']}" + errors["file_names"].append(full_filepath) + errors["messages"].append( + f"The file {full_filepath} failed to transfer. Reason: {line_json['msg']}" + ) + else: + errors["messages"].append(f"ERROR : {line_json['msg']}") + + elif "stats" in line_json and "totalTransfers" in line_json["stats"]: + if line_json["stats"]["totalTransfers"] == 0: + errors[f"nothing_was_transferred_{top_level_folder}"] = True # type:ignore + else: + errors[f"nothing_was_transferred_{top_level_folder}"] = False # type:ignore split_stream[idx] = ( f"{line_json['time'][:19]} {line_json['level'].upper()} : {line_json['msg']}" From 85f90f6b733e9a597fdd8d2ae6032cf35a31490c Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Thu, 23 Oct 2025 15:54:54 +0100 Subject: [PATCH 08/25] Docstrings and linting. --- datashuttle/datashuttle_class.py | 2 +- datashuttle/tui/screens/modal_dialogs.py | 6 +++ datashuttle/utils/custom_types.py | 20 +++++---- datashuttle/utils/data_transfer.py | 3 +- datashuttle/utils/rclone.py | 56 +++++++++++++++++++++--- 5 files changed, 69 insertions(+), 18 deletions(-) diff --git a/datashuttle/datashuttle_class.py b/datashuttle/datashuttle_class.py index 073a3a1cc..f811e9db4 100644 --- a/datashuttle/datashuttle_class.py +++ b/datashuttle/datashuttle_class.py @@ -829,7 +829,7 @@ def _transfer_specific_file_or_folder( stdout, stderr, errors = rclone.parse_rclone_copy_output( top_level_folder, output ) - rclone.log_rclone_output_python_api(stdout, stderr) + rclone.log_stdout_stderr_python_api(stdout, stderr) rclone.log_rclone_copy_errors_api(errors) return errors diff --git a/datashuttle/tui/screens/modal_dialogs.py b/datashuttle/tui/screens/modal_dialogs.py index 1e7512042..45cacf10f 100644 --- a/datashuttle/tui/screens/modal_dialogs.py +++ b/datashuttle/tui/screens/modal_dialogs.py @@ -66,6 +66,12 @@ def __init__( border_color Color of the MessageBox border (e.g. green if the message is positive). + height + The height of the messagebox. + + width + The width of the messagebox. + """ super(MessageBox, self).__init__() diff --git a/datashuttle/utils/custom_types.py b/datashuttle/utils/custom_types.py index f1407d02f..af5c48124 100644 --- a/datashuttle/utils/custom_types.py +++ b/datashuttle/utils/custom_types.py @@ -1,23 +1,25 @@ -from typing import Any, Literal, Tuple, TypeAlias, TypedDict +from __future__ import annotations -DisplayMode: TypeAlias = Literal["error", "warn", "print"] +from typing import Any, Literal, Tuple, TypedDict -TopLevelFolder: TypeAlias = Literal["rawdata", "derivatives"] +DisplayMode = Literal["error", "warn", "print"] -OverwriteExistingFiles: TypeAlias = Literal[ - "never", "always", "if_source_newer" -] +TopLevelFolder = Literal["rawdata", "derivatives"] + +OverwriteExistingFiles = Literal["never", "always", "if_source_newer"] -Prefix: TypeAlias = Literal["sub", "ses"] +Prefix = Literal["sub", "ses"] -InterfaceOutput: TypeAlias = Tuple[bool, Any] +InterfaceOutput = Tuple[bool, Any] -ConnectionMethods: TypeAlias = Literal[ +ConnectionMethods = Literal[ "ssh", "local_filesystem", "gdrive", "aws", "local_only" ] class TransferErrors(TypedDict): + """Type `errors` dictionary (used for collecting `rclone copy` output).""" + file_names: list[str] messages: list[str] nothing_was_transferred_rawdata: bool | None diff --git a/datashuttle/utils/data_transfer.py b/datashuttle/utils/data_transfer.py index 3b36a55ee..c7a021d47 100644 --- a/datashuttle/utils/data_transfer.py +++ b/datashuttle/utils/data_transfer.py @@ -90,6 +90,7 @@ def __init__( self.check_input_arguments() def run(self): + """Run the transfer.""" include_list = self.build_a_list_of_all_files_and_folders_to_transfer() if any(include_list): @@ -113,7 +114,7 @@ def run(self): "Please contact the datashuttle team." ) - rclone.log_rclone_output_python_api(stdout, stderr) + rclone.log_stdout_stderr_python_api(stdout, stderr) else: utils.log_and_message("No files included. None transferred.") diff --git a/datashuttle/utils/rclone.py b/datashuttle/utils/rclone.py index 5b1ffc653..1a89d5875 100644 --- a/datashuttle/utils/rclone.py +++ b/datashuttle/utils/rclone.py @@ -475,8 +475,8 @@ def transfer_data( return output -def log_rclone_output_python_api(stdout, stderr): - """""" +def log_stdout_stderr_python_api(stdout: str, stderr: str) -> None: + """Log `stdout` and `stderr`.""" message = ( f"\n\n************** STDOUT **************\n" f"{stdout}" @@ -488,7 +488,12 @@ def log_rclone_output_python_api(stdout, stderr): def log_rclone_copy_errors_api(errors): - """""" + """Log the `errors` dictionary. + + The `errors` dictionary contains all pertinent information on + issues that occurred when running `rclone copy`. Note this logs + for the API, the TUI display is handled separately. + """ message = "" if errors["nothing_was_transferred_rawdata"] is True: @@ -517,7 +522,12 @@ def log_rclone_copy_errors_api(errors): def parse_rclone_copy_output(top_level_folder, output): - """""" + """Format the `rclone copy` output ready for logging. + + Reformat and combine the string streams and `errors` + dictionary from stdout and stderr output of `rclone copy`. + see `reformat_rclone_copy_output() for details. + """ stdout, out_errors = reformat_rclone_copy_output( output.stdout, top_level_folder=top_level_folder ) @@ -526,9 +536,9 @@ def parse_rclone_copy_output(top_level_folder, output): output.stderr, top_level_folder=top_level_folder ) + # Combine the two `errors` output all_errors = { - "file_names": out_errors["file_names"] - + err_errors["file_names"], # TODO: this is so messy + "file_names": out_errors["file_names"] + err_errors["file_names"], "messages": out_errors["messages"] + err_errors["messages"], "nothing_was_transferred_rawdata": err_errors[ "nothing_was_transferred_rawdata" @@ -544,6 +554,11 @@ def parse_rclone_copy_output(top_level_folder, output): def get_empty_errors_dict() -> TransferErrors: + """Return the `errors` dictionary with default values. + + The `errors` dictionary holds information + about errors which occurred during `rclone copy` transfer. + """ return { "file_names": [], "messages": [], @@ -556,7 +571,34 @@ def reformat_rclone_copy_output( stream: bytes, top_level_folder: TopLevelFolder | None = None, ) -> tuple[str, TransferErrors]: - """:return:""" + """Parse the output of `rclone copy` for convenient error checking. + + Rclone's `copy` command (called with `--use-json-log`) outputs a lot of + information related to the transfer. We dump this in text form to a log + file. However, we also want to grab any key events (errors, or complete + lack of transferred files) so these can be displayed separately. + + This function iterates through all lines in the `rclone copy` output. + This output is typically a mix of string format and json format. + If the line is json-encoded, then we extract important information + and format it to string, and re-insert it into the output. + + In this way, we have a string-format output ready to be + dumped to the logs, as well as an `errors` dictionary containing + details on all key information. + + Returns + ------- + format_stream + The input stream, converted to string and with all + json-formatted lines reformatted as string. This is ready + to be dumped to a log file. + + errors + A dictionary (`TransferErrors`) containing key information + about issues in the transfer. + + """ split_stream = stream.decode("utf-8").split("\n") errors = get_empty_errors_dict() From 5afba6165d3e7d95b466d3ee1ac59f76a3d98244 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Thu, 23 Oct 2025 17:05:02 +0100 Subject: [PATCH 09/25] Tidy up tests, add some docstrings. --- datashuttle/datashuttle_class.py | 1 - pyproject.toml | 1 + .../local_filesystem/test_transfer.py | 72 +++++++++++++++++-- tests/tests_tui/test_tui_transfer.py | 38 ++++++++-- 4 files changed, 97 insertions(+), 15 deletions(-) diff --git a/datashuttle/datashuttle_class.py b/datashuttle/datashuttle_class.py index f811e9db4..868df4f02 100644 --- a/datashuttle/datashuttle_class.py +++ b/datashuttle/datashuttle_class.py @@ -1500,7 +1500,6 @@ def _transfer_entire_project( 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) diff --git a/pyproject.toml b/pyproject.toml index 2e9402411..66abee6c1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -67,6 +67,7 @@ dev = [ "types-setuptools", "pytest-asyncio", "validators", + "filelock" ] [build-system] diff --git a/tests/tests_transfers/local_filesystem/test_transfer.py b/tests/tests_transfers/local_filesystem/test_transfer.py index a6dac479b..805ef1918 100644 --- a/tests/tests_transfers/local_filesystem/test_transfer.py +++ b/tests/tests_transfers/local_filesystem/test_transfer.py @@ -4,6 +4,7 @@ from pathlib import Path import pytest +from filelock import FileLock from datashuttle.configs import canonical_folders from datashuttle.configs.canonical_configs import get_broad_datatypes @@ -647,14 +648,18 @@ def test_errors_variable(self, project, monkeypatch): returned from every transfer function. `errors` is a variable that contains information about any errors that were encountered during transfer. - - DOCS TODO this doesn't matter if its top level folder or whatever """ + # Monkeypatch the error-parsing function so it returns + # predictable values. + import datashuttle + def test_errors(top_level_folder): return { "file_names": [f"{top_level_folder}/hello_world.txt"], "messages": ["how are you?"], + "nothing_was_transferred_rawdata": None, + "nothing_was_transferred_derivatives": None, } def monkeypatch_parse_output(top_level_folder, b): @@ -662,14 +667,13 @@ def monkeypatch_parse_output(top_level_folder, b): stderr = "stderr" return stdout, stderr, test_errors(top_level_folder) - import datashuttle - monkeypatch.setattr( datashuttle.utils.rclone, "parse_rclone_copy_output", monkeypatch_parse_output, ) + # Generate some test files so the transfer runs properly subs, sessions = test_utils.get_default_sub_sessions_to_test() for top_level_folder in ["rawdata", "derivatives"]: @@ -681,10 +685,13 @@ def monkeypatch_parse_output(top_level_folder, b): get_broad_datatypes(), ) + # Run every transfer function and check that + # `errors` is returned correctly. specific_file = ( lambda path_: f"{path_}/rawdata/{subs[0]}/{sessions[0]}/ephys/placeholder_file.txt" ) + # All 'rawdata' functions for func in [ lambda: project.upload_specific_folder_or_file( specific_file(project.get_local_path()) @@ -699,9 +706,12 @@ def monkeypatch_parse_output(top_level_folder, b): ]: assert func() == test_errors("rawdata") + # All 'derivatives' functions for func in [project.upload_derivatives, project.download_derivatives]: assert func() == test_errors("derivatives") + # Entire project functions should merge the errors + # of rawdata and derivatives for func in [ project.upload_entire_project, project.download_entire_project, @@ -712,10 +722,19 @@ def monkeypatch_parse_output(top_level_folder, b): "derivatives/hello_world.txt", ], "messages": ["how are you?", "how are you?"], + "nothing_was_transferred_rawdata": None, + "nothing_was_transferred_derivatives": None, } def test_errors_are_caught_and_logged(self, project): - """""" # TODO DOC OMD TODO! + """ + Create errors in the transfer by locking files, and check + the errors are correctly flagged in logs and `errors`. Also, + perform a transfer where no files are transferred, and check + this is flagged in logs and `errors`. + """ + + # Set up a folder to transfer subs, sessions = test_utils.get_default_sub_sessions_to_test() test_utils.make_and_check_local_project_folders( @@ -726,6 +745,7 @@ def test_errors_are_caught_and_logged(self, project): get_broad_datatypes(), ) + # Lock a file then perform the transfer, causing errors. relative_path = ( Path("rawdata") / subs[0] @@ -737,12 +757,11 @@ def test_errors_are_caught_and_logged(self, project): test_utils.delete_log_files(project.cfg.logging_path) - from filelock import FileLock - lock = FileLock(a_transferred_file, timeout=5) with lock: errors = project.upload_custom("rawdata", "all", "all", "all") + # Check that errors and logs flag the transfer errors assert errors["file_names"] == [relative_path.as_posix()] assert ( "another process has locked a portion of the file" @@ -755,6 +774,45 @@ def test_errors_are_caught_and_logged(self, project): assert "Errors were detected!" in log assert "another process has locked a portion of the file" in log + # Now, we'll perform full transfer so that all future + # transfers do not actually transfera nything + project.upload_entire_project() + + test_utils.delete_log_files(project.cfg.logging_path) + + # Check that it is flagged that no transfer took place for rawdata + errors = project.upload_custom("rawdata", "all", "all", "all") + + assert errors["nothing_was_transferred_rawdata"] is True + assert errors["nothing_was_transferred_derivatives"] is None + + log = test_utils.read_log_file(project.cfg.logging_path) + assert "Note! Nothing was transferred from rawdata!" in log + + test_utils.delete_log_files(project.cfg.logging_path) + + # Check that it is flagged that no transfer took place for derivatives + errors = project.upload_custom("derivatives", "all", "all", "all") + + assert errors["nothing_was_transferred_rawdata"] is None + assert errors["nothing_was_transferred_derivatives"] is True + + log = test_utils.read_log_file(project.cfg.logging_path) + assert "Note! Nothing was transferred from derivatives!" in log + + test_utils.delete_log_files(project.cfg.logging_path) + + # Check that it is flagged that no transfer took place + # for both rawdata and derivatives + errors = project.upload_entire_project() + + assert errors["nothing_was_transferred_rawdata"] is True + assert errors["nothing_was_transferred_derivatives"] is True + + log = test_utils.read_log_file(project.cfg.logging_path) + assert "Note! Nothing was transferred from rawdata!" in log + assert "Note! Nothing was transferred from derivatives!" in log + def get_paths_to_a_local_and_central_file(self, project, top_level_folder): path_to_test_file = ( Path(top_level_folder) diff --git a/tests/tests_tui/test_tui_transfer.py b/tests/tests_tui/test_tui_transfer.py index 44f2d6f99..97f804b78 100644 --- a/tests/tests_tui/test_tui_transfer.py +++ b/tests/tests_tui/test_tui_transfer.py @@ -1,7 +1,11 @@ +from pathlib import Path + import pytest +from filelock import FileLock from datashuttle.configs import canonical_configs from datashuttle.tui.app import TuiApp +from datashuttle.tui.screens.modal_dialogs import MessageBox from .. import test_utils from .tui_base import TuiBase @@ -220,13 +224,19 @@ async def test_transfer_custom( @pytest.mark.asyncio async def test_errors_are_reported_on_pop_up(self, setup_project_paths): - """""" + """ + Check that transfer errors, or the case where no files are transferred, + are displayed properly on the modal dialog that displays following + the transfer. + """ tmp_config_path, tmp_path, project_name = setup_project_paths.values() subs, sessions = test_utils.get_default_sub_sessions_to_test() app = TuiApp() async with app.run_test(size=self.tui_size()) as pilot: + # Set up the project files to transfer and navigate + # to the transfer screen await self.check_and_click_onto_existing_project( pilot, project_name ) @@ -242,8 +252,6 @@ async def test_errors_are_reported_on_pop_up(self, setup_project_paths): "upload", ) - from pathlib import Path - relative_path = ( Path("rawdata") / subs[0] @@ -251,9 +259,10 @@ async def test_errors_are_reported_on_pop_up(self, setup_project_paths): / "ephys" / "placeholder_file.txt" ) - a_transferred_file = project.get_local_path() / relative_path - from filelock import FileLock + # Lock a file and perform the transfer, which will have errors. + # Check the errors are displayed in the pop-up window. + a_transferred_file = project.get_local_path() / relative_path lock = FileLock(a_transferred_file, timeout=5) with lock: @@ -261,8 +270,6 @@ async def test_errors_are_reported_on_pop_up(self, setup_project_paths): pilot, "upload", close_final_messagebox=False ) - from datashuttle.tui.screens.modal_dialogs import MessageBox - assert isinstance(app.screen, MessageBox) displayed_message = app.screen.message @@ -271,6 +278,23 @@ async def test_errors_are_reported_on_pop_up(self, setup_project_paths): " another process has locked a portion of the file" in displayed_message ) + await self.close_messagebox(pilot) + + # Now run a transfer so that all files are transferred, + # then transfer again to check the message displays indicating + # no files were transferred. + await self.run_transfer(pilot, "upload") + + await self.run_transfer( + pilot, "upload", close_final_messagebox=False + ) + + assert ( + "Note! Nothing was transferred from rawdata" + in app.screen.message + ) + + await pilot.pause() async def switch_top_level_folder_select( self, pilot, id, top_level_folder From 9763df882e10b96dc72d4d0f265089911e9765cf Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Thu, 23 Oct 2025 17:16:24 +0100 Subject: [PATCH 10/25] Add a bit more documentation. --- datashuttle/tui/screens/modal_dialogs.py | 6 ++++- datashuttle/utils/rclone.py | 28 +++++++++++++++++++++++- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/datashuttle/tui/screens/modal_dialogs.py b/datashuttle/tui/screens/modal_dialogs.py index 45cacf10f..edcb4cdd2 100644 --- a/datashuttle/tui/screens/modal_dialogs.py +++ b/datashuttle/tui/screens/modal_dialogs.py @@ -237,7 +237,11 @@ def on_button_pressed(self, event: Button.Pressed) -> None: self.dismiss() async def handle_transfer_and_update_ui_when_complete(self) -> None: - """Run the data transfer worker and updates the UI on completion.""" + """Run the data transfer worker and updates the UI on completion. + + Note this function is very similar to `log_rclone_copy_errors_api` + but kept separate for flexibility. + """ try: data_transfer_worker = self.transfer_func() await data_transfer_worker.wait() diff --git a/datashuttle/utils/rclone.py b/datashuttle/utils/rclone.py index 1a89d5875..9e10c65c4 100644 --- a/datashuttle/utils/rclone.py +++ b/datashuttle/utils/rclone.py @@ -493,6 +493,10 @@ def log_rclone_copy_errors_api(errors): The `errors` dictionary contains all pertinent information on issues that occurred when running `rclone copy`. Note this logs for the API, the TUI display is handled separately. + + Note this function is very similar + to `handle_transfer_and_update_ui_when_complete` + but kept separate for flexibility. """ message = "" @@ -516,7 +520,7 @@ def log_rclone_copy_errors_api(errors): message += "\n" if message == "": - message = "No transfer errors were detected.\n" + message = "No errors detected" utils.log_and_message(message, use_rich=True) @@ -558,6 +562,28 @@ def get_empty_errors_dict() -> TransferErrors: The `errors` dictionary holds information about errors which occurred during `rclone copy` transfer. + The dict entries are: + + file_names + A list of file names associated with errors. + + messages + A list of messages associated with errors. For each file name, + there will be an associated message, but it is also possible to + have messages that are not associated with any file name. + + nothing_was_transferred_rawdata + A flag that can take the value `None`, `True` or `False`. + If `None`, this top-level folder was not attempted to be transferred. + If `True`, it was attempted and nothing was transferred. If `False`, + it was attempted and something was transferred. + + nothing_was_transferred_derivatives + See `nothing_was_transferred_rawdata`, this is the equivalent for + the derivatives' folder. + + The rawdata and derivatives flags must be split as some functions + transfer a single, or both, top level folders in one command. """ return { "file_names": [], From 17a51aa71a515dee088a45795ca72fe545e5cc98 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Fri, 24 Oct 2025 13:20:22 +0100 Subject: [PATCH 11/25] Tidy up and fix tests. --- datashuttle/tui/screens/modal_dialogs.py | 4 ++-- datashuttle/utils/rclone.py | 6 +++--- tests/tests_integration/test_logging.py | 2 -- .../local_filesystem/test_transfer.py | 14 ++++---------- tests/tests_tui/test_tui_transfer.py | 15 +++++++++------ 5 files changed, 18 insertions(+), 23 deletions(-) diff --git a/datashuttle/tui/screens/modal_dialogs.py b/datashuttle/tui/screens/modal_dialogs.py index edcb4cdd2..36939cf4a 100644 --- a/datashuttle/tui/screens/modal_dialogs.py +++ b/datashuttle/tui/screens/modal_dialogs.py @@ -263,10 +263,10 @@ async def handle_transfer_and_update_ui_when_complete(self) -> None: ) if errors["nothing_was_transferred_rawdata"] is True: - errors_message += f"[{no_transfer_col}]\nNote! Nothing was transferred from rawdata[/{no_transfer_col}]\n" + errors_message += f"[{no_transfer_col}]\nNote! Nothing was transferred from rawdata.[/{no_transfer_col}]\n" if errors["nothing_was_transferred_derivatives"] is True: - errors_message += f"[{no_transfer_col}]\nNote! Nothing was transferred from derivatives[/{no_transfer_col}]\n" + errors_message += f"[{no_transfer_col}]\nNote! Nothing was transferred from derivatives.[/{no_transfer_col}]\n" if any(errors["messages"]): if errors["file_names"]: diff --git a/datashuttle/utils/rclone.py b/datashuttle/utils/rclone.py index 9e10c65c4..b292211e3 100644 --- a/datashuttle/utils/rclone.py +++ b/datashuttle/utils/rclone.py @@ -501,10 +501,10 @@ def log_rclone_copy_errors_api(errors): message = "" if errors["nothing_was_transferred_rawdata"] is True: - message += "\n\nNote! Nothing was transferred from rawdata!\n" + message += "\n\nNote! Nothing was transferred from rawdata.\n" if errors["nothing_was_transferred_derivatives"] is True: - message += "\n\nNote! Nothing was transferred from derivatives!\n" + message += "\n\nNote! Nothing was transferred from derivatives.\n" if any(errors["messages"]): if any(errors["file_names"]): @@ -795,7 +795,7 @@ def handle_rclone_arguments( extra_arguments_list += [rclone_args("never_overwrite")] elif overwrite == "always": - pass + extra_arguments_list += ["--ignore-times"] elif overwrite == "if_source_newer": extra_arguments_list += [rclone_args("if_source_newer_overwrite")] diff --git a/tests/tests_integration/test_logging.py b/tests/tests_integration/test_logging.py index 4dac348ab..9b5bd4a0b 100644 --- a/tests/tests_integration/test_logging.py +++ b/tests/tests_integration/test_logging.py @@ -291,7 +291,6 @@ def test_logs_upload_and_download( assert "--include" in log assert "sub-11/ses-123/anat/**" in log assert "/central/test_project/rawdata" in log - assert "No transfer errors were detected" in log @pytest.mark.parametrize("upload_or_download", ["upload", "download"]) def test_logs_upload_and_download_folder_or_file( @@ -330,7 +329,6 @@ def test_logs_upload_and_download_folder_or_file( ) assert "sub-001/ses-001" in log assert "Elapsed time" in log - assert "No transfer errors were detected" in log # ---------------------------------------------------------------------------------- # Test temporary logging path diff --git a/tests/tests_transfers/local_filesystem/test_transfer.py b/tests/tests_transfers/local_filesystem/test_transfer.py index 805ef1918..5f8804f50 100644 --- a/tests/tests_transfers/local_filesystem/test_transfer.py +++ b/tests/tests_transfers/local_filesystem/test_transfer.py @@ -774,12 +774,6 @@ def test_errors_are_caught_and_logged(self, project): assert "Errors were detected!" in log assert "another process has locked a portion of the file" in log - # Now, we'll perform full transfer so that all future - # transfers do not actually transfera nything - project.upload_entire_project() - - test_utils.delete_log_files(project.cfg.logging_path) - # Check that it is flagged that no transfer took place for rawdata errors = project.upload_custom("rawdata", "all", "all", "all") @@ -787,7 +781,7 @@ def test_errors_are_caught_and_logged(self, project): assert errors["nothing_was_transferred_derivatives"] is None log = test_utils.read_log_file(project.cfg.logging_path) - assert "Note! Nothing was transferred from rawdata!" in log + assert "Note! Nothing was transferred from rawdata." in log test_utils.delete_log_files(project.cfg.logging_path) @@ -798,7 +792,7 @@ def test_errors_are_caught_and_logged(self, project): assert errors["nothing_was_transferred_derivatives"] is True log = test_utils.read_log_file(project.cfg.logging_path) - assert "Note! Nothing was transferred from derivatives!" in log + assert "Note! Nothing was transferred from derivatives." in log test_utils.delete_log_files(project.cfg.logging_path) @@ -810,8 +804,8 @@ def test_errors_are_caught_and_logged(self, project): assert errors["nothing_was_transferred_derivatives"] is True log = test_utils.read_log_file(project.cfg.logging_path) - assert "Note! Nothing was transferred from rawdata!" in log - assert "Note! Nothing was transferred from derivatives!" in log + assert "Note! Nothing was transferred from rawdata." in log + assert "Note! Nothing was transferred from derivatives." in log def get_paths_to_a_local_and_central_file(self, project, top_level_folder): path_to_test_file = ( diff --git a/tests/tests_tui/test_tui_transfer.py b/tests/tests_tui/test_tui_transfer.py index 97f804b78..19acaf3b2 100644 --- a/tests/tests_tui/test_tui_transfer.py +++ b/tests/tests_tui/test_tui_transfer.py @@ -5,7 +5,6 @@ from datashuttle.configs import canonical_configs from datashuttle.tui.app import TuiApp -from datashuttle.tui.screens.modal_dialogs import MessageBox from .. import test_utils from .tui_base import TuiBase @@ -260,6 +259,14 @@ async def test_errors_are_reported_on_pop_up(self, setup_project_paths): / "placeholder_file.txt" ) + # Now run a transfer so that all files are transferred, + await self.run_transfer( + pilot, "upload", close_final_messagebox=False + ) + + assert "No errors detected" in app.screen.message + await self.close_messagebox(pilot) + # Lock a file and perform the transfer, which will have errors. # Check the errors are displayed in the pop-up window. a_transferred_file = project.get_local_path() / relative_path @@ -270,7 +277,6 @@ async def test_errors_are_reported_on_pop_up(self, setup_project_paths): pilot, "upload", close_final_messagebox=False ) - assert isinstance(app.screen, MessageBox) displayed_message = app.screen.message assert relative_path.as_posix() in displayed_message @@ -280,11 +286,8 @@ async def test_errors_are_reported_on_pop_up(self, setup_project_paths): ) await self.close_messagebox(pilot) - # Now run a transfer so that all files are transferred, - # then transfer again to check the message displays indicating + # Transfer again to check the message displays indicating # no files were transferred. - await self.run_transfer(pilot, "upload") - await self.run_transfer( pilot, "upload", close_final_messagebox=False ) From 7f73b3fafb9c9c85e67044a21efb10400944bcfc Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Fri, 24 Oct 2025 14:24:09 +0100 Subject: [PATCH 12/25] Fix tests...again. --- datashuttle/utils/rclone.py | 2 +- tests/tests_tui/test_tui_transfer.py | 8 -------- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/datashuttle/utils/rclone.py b/datashuttle/utils/rclone.py index b292211e3..14c247855 100644 --- a/datashuttle/utils/rclone.py +++ b/datashuttle/utils/rclone.py @@ -795,7 +795,7 @@ def handle_rclone_arguments( extra_arguments_list += [rclone_args("never_overwrite")] elif overwrite == "always": - extra_arguments_list += ["--ignore-times"] + pass elif overwrite == "if_source_newer": extra_arguments_list += [rclone_args("if_source_newer_overwrite")] diff --git a/tests/tests_tui/test_tui_transfer.py b/tests/tests_tui/test_tui_transfer.py index 19acaf3b2..499ff0ea1 100644 --- a/tests/tests_tui/test_tui_transfer.py +++ b/tests/tests_tui/test_tui_transfer.py @@ -259,14 +259,6 @@ async def test_errors_are_reported_on_pop_up(self, setup_project_paths): / "placeholder_file.txt" ) - # Now run a transfer so that all files are transferred, - await self.run_transfer( - pilot, "upload", close_final_messagebox=False - ) - - assert "No errors detected" in app.screen.message - await self.close_messagebox(pilot) - # Lock a file and perform the transfer, which will have errors. # Check the errors are displayed in the pop-up window. a_transferred_file = project.get_local_path() / relative_path From 4a562a85c62314fa8cc2a3d5ec4c23ba27e330a7 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Fri, 24 Oct 2025 16:32:09 +0100 Subject: [PATCH 13/25] Fix tests on linux. --- tests/test_utils.py | 20 +++++++++++++ .../local_filesystem/test_transfer.py | 29 +++++++++++++------ tests/tests_tui/test_tui_transfer.py | 22 +++++++------- 3 files changed, 52 insertions(+), 19 deletions(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index bb38c5851..54f80a21b 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -6,6 +6,8 @@ import os import pathlib import shutil +import threading +import time import warnings from os.path import join from pathlib import Path @@ -728,3 +730,21 @@ def mock_get_datashuttle_path(): "datashuttle.configs.canonical_folders.get_datashuttle_path", mock_get_datashuttle_path, ) + + +def lock_a_file(file_path, duration=5): + """""" + + def continually_write_to_file(path, duration): + end_time = time.time() + duration + with open(path, "a") as f: + while time.time() < end_time: + f.write("LOCKED\n") + f.flush() + + t = threading.Thread( + target=continually_write_to_file, args=(file_path, duration) + ) + t.start() + + return t diff --git a/tests/tests_transfers/local_filesystem/test_transfer.py b/tests/tests_transfers/local_filesystem/test_transfer.py index 5f8804f50..3a7767642 100644 --- a/tests/tests_transfers/local_filesystem/test_transfer.py +++ b/tests/tests_transfers/local_filesystem/test_transfer.py @@ -4,7 +4,6 @@ from pathlib import Path import pytest -from filelock import FileLock from datashuttle.configs import canonical_folders from datashuttle.configs.canonical_configs import get_broad_datatypes @@ -757,22 +756,34 @@ def test_errors_are_caught_and_logged(self, project): test_utils.delete_log_files(project.cfg.logging_path) - lock = FileLock(a_transferred_file, timeout=5) - with lock: - errors = project.upload_custom("rawdata", "all", "all", "all") + def lock_a_file(path, duration=5): + # Keep appending for a few seconds to simulate a mid-transfer write + end_time = time.time() + duration + with open(path, "a") as f: + while time.time() < end_time: + f.write("LOCKED / corrupted mid-transfer\n") + f.flush() + # time.sleep(0.01) # small delay to simulate ongoing write + + test_utils.delete_log_files(project.cfg.logging_path) + + thread = test_utils.lock_a_file(a_transferred_file) + errors = project.upload_custom("rawdata", "all", "all", "all") + thread.join() # Check that errors and logs flag the transfer errors assert errors["file_names"] == [relative_path.as_posix()] - assert ( - "another process has locked a portion of the file" - in errors["messages"][0] - ) + assert "size changed" in errors["messages"][0] log = test_utils.read_log_file(project.cfg.logging_path) assert errors["file_names"][0] in log assert "Errors were detected!" in log - assert "another process has locked a portion of the file" in log + assert "size changed" in log + + # now just upload everything + project.upload_entire_project() + test_utils.delete_log_files(project.cfg.logging_path) # Check that it is flagged that no transfer took place for rawdata errors = project.upload_custom("rawdata", "all", "all", "all") diff --git a/tests/tests_tui/test_tui_transfer.py b/tests/tests_tui/test_tui_transfer.py index 499ff0ea1..3185af191 100644 --- a/tests/tests_tui/test_tui_transfer.py +++ b/tests/tests_tui/test_tui_transfer.py @@ -1,7 +1,6 @@ from pathlib import Path import pytest -from filelock import FileLock from datashuttle.configs import canonical_configs from datashuttle.tui.app import TuiApp @@ -263,23 +262,26 @@ async def test_errors_are_reported_on_pop_up(self, setup_project_paths): # Check the errors are displayed in the pop-up window. a_transferred_file = project.get_local_path() / relative_path - lock = FileLock(a_transferred_file, timeout=5) - with lock: - await self.run_transfer( - pilot, "upload", close_final_messagebox=False - ) + thread = test_utils.lock_a_file(a_transferred_file, duration=20) + + await self.run_transfer( + pilot, "upload", close_final_messagebox=False + ) + + thread.join() displayed_message = app.screen.message assert relative_path.as_posix() in displayed_message - assert ( - " another process has locked a portion of the file" - in displayed_message - ) + + assert "size changed" in displayed_message + await self.close_messagebox(pilot) # Transfer again to check the message displays indicating # no files were transferred. + await self.run_transfer(pilot, "upload") + await self.run_transfer( pilot, "upload", close_final_messagebox=False ) From 55ef24241b4e96a10b56805df0ca220797165e45 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Fri, 24 Oct 2025 17:41:52 +0100 Subject: [PATCH 14/25] Playing around windows only... --- .github/workflows/code_test_and_deploy.yml | 26 +------------------ .../local_filesystem/test_transfer.py | 13 +++------- 2 files changed, 5 insertions(+), 34 deletions(-) diff --git a/.github/workflows/code_test_and_deploy.yml b/.github/workflows/code_test_and_deploy.yml index 950c9957c..a7a9f00f0 100644 --- a/.github/workflows/code_test_and_deploy.yml +++ b/.github/workflows/code_test_and_deploy.yml @@ -68,33 +68,9 @@ jobs: # run SSH tests only on Linux because Windows and macOS # are already run within a virtual container and so cannot # run Linux containers because nested containerisation is disabled. - - name: Test SSH (Linux only) - if: runner.os == 'Linux' - run: | - sudo service mysql stop # free up port 3306 for ssh tests - pytest tests/tests_transfers/ssh - - - name: Test Google Drive - env: - GDRIVE_CLIENT_ID: ${{ secrets.GDRIVE_CLIENT_ID }} - GDRIVE_CLIENT_SECRET: ${{ secrets.GDRIVE_CLIENT_SECRET }} - GDRIVE_ROOT_FOLDER_ID: ${{ secrets.GDRIVE_ROOT_FOLDER_ID }} - GDRIVE_CONFIG_TOKEN: ${{ secrets.GDRIVE_CONFIG_TOKEN }} - run: | - pytest tests/tests_transfers/gdrive - - - name: Test AWS - env: - AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }} - AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }} - AWS_REGION: ${{ secrets.AWS_REGION }} - AWS_BUCKET_NAME: ${{ secrets.AWS_BUCKET_NAME }} - run: | - pytest tests/tests_transfers/aws - - name: All Other Tests run: | - pytest --ignore=tests/tests_transfers/ssh --ignore=tests/tests_transfers/gdrive --ignore=tests/tests_transfers/aws + pytest -k test_transfer build_sdist_wheels: diff --git a/tests/tests_transfers/local_filesystem/test_transfer.py b/tests/tests_transfers/local_filesystem/test_transfer.py index 3a7767642..9a8a08f5e 100644 --- a/tests/tests_transfers/local_filesystem/test_transfer.py +++ b/tests/tests_transfers/local_filesystem/test_transfer.py @@ -1,4 +1,5 @@ import os +import platform import re import time from pathlib import Path @@ -725,6 +726,9 @@ def monkeypatch_parse_output(top_level_folder, b): "nothing_was_transferred_derivatives": None, } + @pytest.mark.skipif( + platform.system() != "Windows", reason="Only run on windows." + ) def test_errors_are_caught_and_logged(self, project): """ Create errors in the transfer by locking files, and check @@ -756,15 +760,6 @@ def test_errors_are_caught_and_logged(self, project): test_utils.delete_log_files(project.cfg.logging_path) - def lock_a_file(path, duration=5): - # Keep appending for a few seconds to simulate a mid-transfer write - end_time = time.time() + duration - with open(path, "a") as f: - while time.time() < end_time: - f.write("LOCKED / corrupted mid-transfer\n") - f.flush() - # time.sleep(0.01) # small delay to simulate ongoing write - test_utils.delete_log_files(project.cfg.logging_path) thread = test_utils.lock_a_file(a_transferred_file) From e04c8ae40dd543d5f3ceca936af5e79c90e289e5 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Mon, 27 Oct 2025 18:08:53 +0000 Subject: [PATCH 15/25] Tests, different cases for locking file on windows vs. macos/linux. --- .../local_filesystem/test_transfer.py | 18 +++++++++---- tests/tests_tui/test_tui_transfer.py | 26 ++++++++++++++----- 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/tests/tests_transfers/local_filesystem/test_transfer.py b/tests/tests_transfers/local_filesystem/test_transfer.py index 9a8a08f5e..76b034911 100644 --- a/tests/tests_transfers/local_filesystem/test_transfer.py +++ b/tests/tests_transfers/local_filesystem/test_transfer.py @@ -5,6 +5,7 @@ from pathlib import Path import pytest +from filelock import FileLock from datashuttle.configs import canonical_folders from datashuttle.configs.canonical_configs import get_broad_datatypes @@ -762,19 +763,26 @@ def test_errors_are_caught_and_logged(self, project): test_utils.delete_log_files(project.cfg.logging_path) - thread = test_utils.lock_a_file(a_transferred_file) - errors = project.upload_custom("rawdata", "all", "all", "all") - thread.join() + if platform.system() == "Windows": + lock = FileLock(a_transferred_file, timeout=5) + with lock: + errors = project.upload_custom("rawdata", "all", "all", "all") + error_message = "because another process has locked " + else: + thread = test_utils.lock_a_file(a_transferred_file) + errors = project.upload_custom("rawdata", "all", "all", "all") + thread.join() + error_message = "size changed" # Check that errors and logs flag the transfer errors assert errors["file_names"] == [relative_path.as_posix()] - assert "size changed" in errors["messages"][0] + assert error_message in errors["messages"][0] log = test_utils.read_log_file(project.cfg.logging_path) assert errors["file_names"][0] in log assert "Errors were detected!" in log - assert "size changed" in log + assert error_message in log # now just upload everything project.upload_entire_project() diff --git a/tests/tests_tui/test_tui_transfer.py b/tests/tests_tui/test_tui_transfer.py index 3185af191..c2d9349b4 100644 --- a/tests/tests_tui/test_tui_transfer.py +++ b/tests/tests_tui/test_tui_transfer.py @@ -1,6 +1,8 @@ +import platform from pathlib import Path import pytest +from filelock import FileLock from datashuttle.configs import canonical_configs from datashuttle.tui.app import TuiApp @@ -262,19 +264,29 @@ async def test_errors_are_reported_on_pop_up(self, setup_project_paths): # Check the errors are displayed in the pop-up window. a_transferred_file = project.get_local_path() / relative_path - thread = test_utils.lock_a_file(a_transferred_file, duration=20) - - await self.run_transfer( - pilot, "upload", close_final_messagebox=False - ) + if platform.system() == "Windows": + lock = FileLock(a_transferred_file, timeout=5) + with lock: + await self.run_transfer( + pilot, "upload", close_final_messagebox=False + ) + error_message = "because another process has locked " + else: + thread = test_utils.lock_a_file( + a_transferred_file, duration=20 + ) - thread.join() + await self.run_transfer( + pilot, "upload", close_final_messagebox=False + ) + thread.join() + error_message = "size changed" displayed_message = app.screen.message assert relative_path.as_posix() in displayed_message - assert "size changed" in displayed_message + assert error_message in displayed_message await self.close_messagebox(pilot) From 67727363295e5c1d2e5bc012c3f3f7c383bf116a Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Wed, 29 Oct 2025 17:46:11 +0000 Subject: [PATCH 16/25] Minor reformating on error messages. --- datashuttle/tui/screens/modal_dialogs.py | 10 +++++----- datashuttle/utils/rclone.py | 4 ++-- .../tests_transfers/local_filesystem/test_transfer.py | 8 ++++---- tests/tests_tui/test_tui_transfer.py | 5 +---- 4 files changed, 12 insertions(+), 15 deletions(-) diff --git a/datashuttle/tui/screens/modal_dialogs.py b/datashuttle/tui/screens/modal_dialogs.py index 36939cf4a..9fb48b244 100644 --- a/datashuttle/tui/screens/modal_dialogs.py +++ b/datashuttle/tui/screens/modal_dialogs.py @@ -263,19 +263,19 @@ async def handle_transfer_and_update_ui_when_complete(self) -> None: ) if errors["nothing_was_transferred_rawdata"] is True: - errors_message += f"[{no_transfer_col}]\nNote! Nothing was transferred from rawdata.[/{no_transfer_col}]\n" + errors_message += f"[{no_transfer_col}]\nNothing was transferred from rawdata.[/{no_transfer_col}]\n" if errors["nothing_was_transferred_derivatives"] is True: - errors_message += f"[{no_transfer_col}]\nNote! Nothing was transferred from derivatives.[/{no_transfer_col}]\n" + errors_message += f"[{no_transfer_col}]\nNothing was transferred from derivatives.[/{no_transfer_col}]\n" if any(errors["messages"]): if errors["file_names"]: errors_message += ( - "[red]Errors detected! in files:[/red]\n" + "\n[red]Errors detected! in files:[/red]\n" ) errors_message += "\n".join(errors["file_names"]) else: - errors_message += "[red]Errors detected![/red]" + errors_message += "\n[red]Errors detected![/red]" errors_message += ( "[red]\n\nThe error messages are:[/red]\n" ) @@ -287,7 +287,7 @@ async def handle_transfer_and_update_ui_when_complete(self) -> None: message = ( f"Transfer finished.\n" - f"{errors_message}\n" + f"{errors_message}\n\n" f"Check the most recent logs for full details." ) diff --git a/datashuttle/utils/rclone.py b/datashuttle/utils/rclone.py index 14c247855..5a85c6693 100644 --- a/datashuttle/utils/rclone.py +++ b/datashuttle/utils/rclone.py @@ -501,10 +501,10 @@ def log_rclone_copy_errors_api(errors): message = "" if errors["nothing_was_transferred_rawdata"] is True: - message += "\n\nNote! Nothing was transferred from rawdata.\n" + message += "\n\nNothing was transferred from rawdata.\n" if errors["nothing_was_transferred_derivatives"] is True: - message += "\n\nNote! Nothing was transferred from derivatives.\n" + message += "\n\nNothing was transferred from derivatives.\n" if any(errors["messages"]): if any(errors["file_names"]): diff --git a/tests/tests_transfers/local_filesystem/test_transfer.py b/tests/tests_transfers/local_filesystem/test_transfer.py index 76b034911..a6e8a0cd7 100644 --- a/tests/tests_transfers/local_filesystem/test_transfer.py +++ b/tests/tests_transfers/local_filesystem/test_transfer.py @@ -795,7 +795,7 @@ def test_errors_are_caught_and_logged(self, project): assert errors["nothing_was_transferred_derivatives"] is None log = test_utils.read_log_file(project.cfg.logging_path) - assert "Note! Nothing was transferred from rawdata." in log + assert "Nothing was transferred from rawdata." in log test_utils.delete_log_files(project.cfg.logging_path) @@ -806,7 +806,7 @@ def test_errors_are_caught_and_logged(self, project): assert errors["nothing_was_transferred_derivatives"] is True log = test_utils.read_log_file(project.cfg.logging_path) - assert "Note! Nothing was transferred from derivatives." in log + assert "Nothing was transferred from derivatives." in log test_utils.delete_log_files(project.cfg.logging_path) @@ -818,8 +818,8 @@ def test_errors_are_caught_and_logged(self, project): assert errors["nothing_was_transferred_derivatives"] is True log = test_utils.read_log_file(project.cfg.logging_path) - assert "Note! Nothing was transferred from rawdata." in log - assert "Note! Nothing was transferred from derivatives." in log + assert "Nothing was transferred from rawdata." in log + assert "Nothing was transferred from derivatives." in log def get_paths_to_a_local_and_central_file(self, project, top_level_folder): path_to_test_file = ( diff --git a/tests/tests_tui/test_tui_transfer.py b/tests/tests_tui/test_tui_transfer.py index c2d9349b4..67a46606a 100644 --- a/tests/tests_tui/test_tui_transfer.py +++ b/tests/tests_tui/test_tui_transfer.py @@ -298,10 +298,7 @@ async def test_errors_are_reported_on_pop_up(self, setup_project_paths): pilot, "upload", close_final_messagebox=False ) - assert ( - "Note! Nothing was transferred from rawdata" - in app.screen.message - ) + assert "Nothing was transferred from rawdata" in app.screen.message await pilot.pause() From e2de47743e008cc86972adc6d03b2a28ef271d0e Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Mon, 27 Oct 2025 20:42:53 +0000 Subject: [PATCH 17/25] Revert worlfkow changes. --- .github/workflows/code_test_and_deploy.yml | 26 +++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/.github/workflows/code_test_and_deploy.yml b/.github/workflows/code_test_and_deploy.yml index a7a9f00f0..950c9957c 100644 --- a/.github/workflows/code_test_and_deploy.yml +++ b/.github/workflows/code_test_and_deploy.yml @@ -68,9 +68,33 @@ jobs: # run SSH tests only on Linux because Windows and macOS # are already run within a virtual container and so cannot # run Linux containers because nested containerisation is disabled. + - name: Test SSH (Linux only) + if: runner.os == 'Linux' + run: | + sudo service mysql stop # free up port 3306 for ssh tests + pytest tests/tests_transfers/ssh + + - name: Test Google Drive + env: + GDRIVE_CLIENT_ID: ${{ secrets.GDRIVE_CLIENT_ID }} + GDRIVE_CLIENT_SECRET: ${{ secrets.GDRIVE_CLIENT_SECRET }} + GDRIVE_ROOT_FOLDER_ID: ${{ secrets.GDRIVE_ROOT_FOLDER_ID }} + GDRIVE_CONFIG_TOKEN: ${{ secrets.GDRIVE_CONFIG_TOKEN }} + run: | + pytest tests/tests_transfers/gdrive + + - name: Test AWS + env: + AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }} + AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }} + AWS_REGION: ${{ secrets.AWS_REGION }} + AWS_BUCKET_NAME: ${{ secrets.AWS_BUCKET_NAME }} + run: | + pytest tests/tests_transfers/aws + - name: All Other Tests run: | - pytest -k test_transfer + pytest --ignore=tests/tests_transfers/ssh --ignore=tests/tests_transfers/gdrive --ignore=tests/tests_transfers/aws build_sdist_wheels: From 3153bff69f4cae4538d90b0030a905a32fca5798 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Mon, 3 Nov 2025 16:43:41 +0000 Subject: [PATCH 18/25] Adjust tests to investigate failures. --- .github/workflows/code_test_and_deploy.yml | 51 +++++++++++----------- 1 file changed, 26 insertions(+), 25 deletions(-) diff --git a/.github/workflows/code_test_and_deploy.yml b/.github/workflows/code_test_and_deploy.yml index 950c9957c..30eaf085a 100644 --- a/.github/workflows/code_test_and_deploy.yml +++ b/.github/workflows/code_test_and_deploy.yml @@ -68,33 +68,34 @@ jobs: # run SSH tests only on Linux because Windows and macOS # are already run within a virtual container and so cannot # run Linux containers because nested containerisation is disabled. - - name: Test SSH (Linux only) - if: runner.os == 'Linux' - run: | - sudo service mysql stop # free up port 3306 for ssh tests - pytest tests/tests_transfers/ssh - - - name: Test Google Drive - env: - GDRIVE_CLIENT_ID: ${{ secrets.GDRIVE_CLIENT_ID }} - GDRIVE_CLIENT_SECRET: ${{ secrets.GDRIVE_CLIENT_SECRET }} - GDRIVE_ROOT_FOLDER_ID: ${{ secrets.GDRIVE_ROOT_FOLDER_ID }} - GDRIVE_CONFIG_TOKEN: ${{ secrets.GDRIVE_CONFIG_TOKEN }} - run: | - pytest tests/tests_transfers/gdrive - - - name: Test AWS - env: - AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }} - AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }} - AWS_REGION: ${{ secrets.AWS_REGION }} - AWS_BUCKET_NAME: ${{ secrets.AWS_BUCKET_NAME }} - run: | - pytest tests/tests_transfers/aws - +# - name: Test SSH (Linux only) +# if: runner.os == 'Linux' +# run: | +# sudo service mysql stop # free up port 3306 for ssh tests +# pytest tests/tests_transfers/ssh +# +# - name: Test Google Drive +# env: +# GDRIVE_CLIENT_ID: ${{ secrets.GDRIVE_CLIENT_ID }} +# GDRIVE_CLIENT_SECRET: ${{ secrets.GDRIVE_CLIENT_SECRET }} +# GDRIVE_ROOT_FOLDER_ID: ${{ secrets.GDRIVE_ROOT_FOLDER_ID }} +# GDRIVE_CONFIG_TOKEN: ${{ secrets.GDRIVE_CONFIG_TOKEN }} +# run: | +# pytest tests/tests_transfers/gdrive +# +# - name: Test AWS +# env: +# AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }} +# AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }} +# AWS_REGION: ${{ secrets.AWS_REGION }} +# AWS_BUCKET_NAME: ${{ secrets.AWS_BUCKET_NAME }} +# run: | +# pytest tests/tests_transfers/aws +# - name: All Other Tests run: | - pytest --ignore=tests/tests_transfers/ssh --ignore=tests/tests_transfers/gdrive --ignore=tests/tests_transfers/aws + pytest -k test_transfer +# --ignore=tests/tests_transfers/ssh --ignore=tests/tests_transfers/gdrive --ignore=tests/tests_transfers/aws build_sdist_wheels: From 47a8a909f93222ea500eb6bf139d1963ce1b6311 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Mon, 3 Nov 2025 17:30:18 +0000 Subject: [PATCH 19/25] Fix path separator issues. --- tests/tests_transfers/local_filesystem/test_transfer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tests_transfers/local_filesystem/test_transfer.py b/tests/tests_transfers/local_filesystem/test_transfer.py index a6e8a0cd7..7710899ff 100644 --- a/tests/tests_transfers/local_filesystem/test_transfer.py +++ b/tests/tests_transfers/local_filesystem/test_transfer.py @@ -780,7 +780,7 @@ def test_errors_are_caught_and_logged(self, project): log = test_utils.read_log_file(project.cfg.logging_path) - assert errors["file_names"][0] in log + assert str(Path(errors["file_names"][0])) in log assert "Errors were detected!" in log assert error_message in log From 826534da17c30acad24b1d975bf4e9e7b3a81f0e Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Mon, 3 Nov 2025 18:21:06 +0000 Subject: [PATCH 20/25] Fix newline. --- datashuttle/utils/ssh.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datashuttle/utils/ssh.py b/datashuttle/utils/ssh.py index c7d6256a4..b31379c1c 100644 --- a/datashuttle/utils/ssh.py +++ b/datashuttle/utils/ssh.py @@ -210,7 +210,7 @@ def connect_client( f"3) The central_host_id: {cfg['central_host_id']} is" f" correct.\n" f"4) The central username:" - f" {cfg['central_host_username']}, and password are correct." + f" {cfg['central_host_username']}, and password are correct.\n" f"Original error: {e}", ConnectionError, ) From bc27a762ac807ac9b47e8e2ba0174d1679e0f09a Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Mon, 3 Nov 2025 18:23:38 +0000 Subject: [PATCH 21/25] Add back all tests. --- .github/workflows/code_test_and_deploy.yml | 51 +++++++++++----------- 1 file changed, 25 insertions(+), 26 deletions(-) diff --git a/.github/workflows/code_test_and_deploy.yml b/.github/workflows/code_test_and_deploy.yml index 30eaf085a..950c9957c 100644 --- a/.github/workflows/code_test_and_deploy.yml +++ b/.github/workflows/code_test_and_deploy.yml @@ -68,34 +68,33 @@ jobs: # run SSH tests only on Linux because Windows and macOS # are already run within a virtual container and so cannot # run Linux containers because nested containerisation is disabled. -# - name: Test SSH (Linux only) -# if: runner.os == 'Linux' -# run: | -# sudo service mysql stop # free up port 3306 for ssh tests -# pytest tests/tests_transfers/ssh -# -# - name: Test Google Drive -# env: -# GDRIVE_CLIENT_ID: ${{ secrets.GDRIVE_CLIENT_ID }} -# GDRIVE_CLIENT_SECRET: ${{ secrets.GDRIVE_CLIENT_SECRET }} -# GDRIVE_ROOT_FOLDER_ID: ${{ secrets.GDRIVE_ROOT_FOLDER_ID }} -# GDRIVE_CONFIG_TOKEN: ${{ secrets.GDRIVE_CONFIG_TOKEN }} -# run: | -# pytest tests/tests_transfers/gdrive -# -# - name: Test AWS -# env: -# AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }} -# AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }} -# AWS_REGION: ${{ secrets.AWS_REGION }} -# AWS_BUCKET_NAME: ${{ secrets.AWS_BUCKET_NAME }} -# run: | -# pytest tests/tests_transfers/aws -# + - name: Test SSH (Linux only) + if: runner.os == 'Linux' + run: | + sudo service mysql stop # free up port 3306 for ssh tests + pytest tests/tests_transfers/ssh + + - name: Test Google Drive + env: + GDRIVE_CLIENT_ID: ${{ secrets.GDRIVE_CLIENT_ID }} + GDRIVE_CLIENT_SECRET: ${{ secrets.GDRIVE_CLIENT_SECRET }} + GDRIVE_ROOT_FOLDER_ID: ${{ secrets.GDRIVE_ROOT_FOLDER_ID }} + GDRIVE_CONFIG_TOKEN: ${{ secrets.GDRIVE_CONFIG_TOKEN }} + run: | + pytest tests/tests_transfers/gdrive + + - name: Test AWS + env: + AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }} + AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }} + AWS_REGION: ${{ secrets.AWS_REGION }} + AWS_BUCKET_NAME: ${{ secrets.AWS_BUCKET_NAME }} + run: | + pytest tests/tests_transfers/aws + - name: All Other Tests run: | - pytest -k test_transfer -# --ignore=tests/tests_transfers/ssh --ignore=tests/tests_transfers/gdrive --ignore=tests/tests_transfers/aws + pytest --ignore=tests/tests_transfers/ssh --ignore=tests/tests_transfers/gdrive --ignore=tests/tests_transfers/aws build_sdist_wheels: From d4834c5ec8cf76eb2616d9af2d1e5b7d1f13005f Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Mon, 3 Nov 2025 23:35:40 +0000 Subject: [PATCH 22/25] Try again! to fix these weird test failures. --- datashuttle/utils/rclone.py | 5 ++++- tests/tests_transfers/local_filesystem/test_transfer.py | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/datashuttle/utils/rclone.py b/datashuttle/utils/rclone.py index 5a85c6693..e87deb779 100644 --- a/datashuttle/utils/rclone.py +++ b/datashuttle/utils/rclone.py @@ -12,6 +12,7 @@ import shlex import subprocess import tempfile +from pathlib import Path from subprocess import CompletedProcess from datashuttle.configs import canonical_configs @@ -637,7 +638,9 @@ def reformat_rclone_copy_output( if line_json["level"] in ["error", "critical"]: if "object" in line_json: - full_filepath = f"{top_level_folder}/{line_json['object']}" + full_filepath = Path( + f"{top_level_folder}/{line_json['object']}" + ).as_posix() errors["file_names"].append(full_filepath) errors["messages"].append( f"The file {full_filepath} failed to transfer. Reason: {line_json['msg']}" diff --git a/tests/tests_transfers/local_filesystem/test_transfer.py b/tests/tests_transfers/local_filesystem/test_transfer.py index 7710899ff..b97ab819a 100644 --- a/tests/tests_transfers/local_filesystem/test_transfer.py +++ b/tests/tests_transfers/local_filesystem/test_transfer.py @@ -780,7 +780,7 @@ def test_errors_are_caught_and_logged(self, project): log = test_utils.read_log_file(project.cfg.logging_path) - assert str(Path(errors["file_names"][0])) in log + assert Path(errors["file_names"][0]).as_posix() in log assert "Errors were detected!" in log assert error_message in log From 06c77c864cda6dfb562a74afa9f3e2543cbe4b29 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Wed, 5 Nov 2025 17:03:09 +0000 Subject: [PATCH 23/25] Try moving to logging module to take advantage of the logging test machinery. --- .github/workflows/code_test_and_deploy.yml | 49 +++++----- tests/tests_integration/test_logging.py | 94 ++++++++++++++++++ .../local_filesystem/test_transfer.py | 96 ------------------- 3 files changed, 119 insertions(+), 120 deletions(-) diff --git a/.github/workflows/code_test_and_deploy.yml b/.github/workflows/code_test_and_deploy.yml index 950c9957c..3114c59e0 100644 --- a/.github/workflows/code_test_and_deploy.yml +++ b/.github/workflows/code_test_and_deploy.yml @@ -68,33 +68,34 @@ jobs: # run SSH tests only on Linux because Windows and macOS # are already run within a virtual container and so cannot # run Linux containers because nested containerisation is disabled. - - name: Test SSH (Linux only) - if: runner.os == 'Linux' - run: | - sudo service mysql stop # free up port 3306 for ssh tests - pytest tests/tests_transfers/ssh - - - name: Test Google Drive - env: - GDRIVE_CLIENT_ID: ${{ secrets.GDRIVE_CLIENT_ID }} - GDRIVE_CLIENT_SECRET: ${{ secrets.GDRIVE_CLIENT_SECRET }} - GDRIVE_ROOT_FOLDER_ID: ${{ secrets.GDRIVE_ROOT_FOLDER_ID }} - GDRIVE_CONFIG_TOKEN: ${{ secrets.GDRIVE_CONFIG_TOKEN }} - run: | - pytest tests/tests_transfers/gdrive - - - name: Test AWS - env: - AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }} - AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }} - AWS_REGION: ${{ secrets.AWS_REGION }} - AWS_BUCKET_NAME: ${{ secrets.AWS_BUCKET_NAME }} - run: | - pytest tests/tests_transfers/aws +# - name: Test SSH (Linux only) +# if: runner.os == 'Linux' +# run: | +# sudo service mysql stop # free up port 3306 for ssh tests +# pytest tests/tests_transfers/ssh + +# - name: Test Google Drive +# env: +# GDRIVE_CLIENT_ID: ${{ secrets.GDRIVE_CLIENT_ID }} +# GDRIVE_CLIENT_SECRET: ${{ secrets.GDRIVE_CLIENT_SECRET }} +# GDRIVE_ROOT_FOLDER_ID: ${{ secrets.GDRIVE_ROOT_FOLDER_ID }} +# GDRIVE_CONFIG_TOKEN: ${{ secrets.GDRIVE_CONFIG_TOKEN }} +# run: | +# pytest tests/tests_transfers/gdrive + +# - name: Test AWS +# env: +# AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }} +# AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }} +# AWS_REGION: ${{ secrets.AWS_REGION }} +# AWS_BUCKET_NAME: ${{ secrets.AWS_BUCKET_NAME }} +# run: | +# pytest tests/tests_transfers/aws - name: All Other Tests run: | - pytest --ignore=tests/tests_transfers/ssh --ignore=tests/tests_transfers/gdrive --ignore=tests/tests_transfers/aws + pytest -k test_transfers +# --ignore=tests/tests_transfers/ssh --ignore=tests/tests_transfers/gdrive --ignore=tests/tests_transfers/aws build_sdist_wheels: diff --git a/tests/tests_integration/test_logging.py b/tests/tests_integration/test_logging.py index 9b5bd4a0b..233ad3d0b 100644 --- a/tests/tests_integration/test_logging.py +++ b/tests/tests_integration/test_logging.py @@ -1,12 +1,15 @@ import glob import logging import os +import platform import re from pathlib import Path import pytest +from filelock import FileLock from datashuttle.configs import canonical_configs +from datashuttle.configs.canonical_configs import get_broad_datatypes from datashuttle.configs.canonical_tags import tags from datashuttle.utils import ds_logger from datashuttle.utils.custom_exceptions import ( @@ -482,3 +485,94 @@ def test_validate_names_against_project_logging(self, project): assert "ERROR" in log assert str(e.value) in log + + def test_errors_are_caught_and_logged(self, project): + """ + Create errors in the transfer by locking files, and check + the errors are correctly flagged in logs and `errors`. Also, + perform a transfer where no files are transferred, and check + this is flagged in logs and `errors`. + """ + + # Set up a folder to transfer + subs, sessions = test_utils.get_default_sub_sessions_to_test() + + test_utils.make_and_check_local_project_folders( + project, + "rawdata", + subs, + sessions, + get_broad_datatypes(), + ) + + # Lock a file then perform the transfer, causing errors. + relative_path = ( + Path("rawdata") + / subs[0] + / sessions[0] + / "ephys" + / "placeholder_file.txt" + ) + a_transferred_file = project.get_local_path() / relative_path + + test_utils.delete_log_files(project.cfg.logging_path) + + test_utils.delete_log_files(project.cfg.logging_path) + + if platform.system() == "Windows": + lock = FileLock(a_transferred_file, timeout=5) + with lock: + errors = project.upload_custom("rawdata", "all", "all", "all") + error_message = "because another process has locked " + else: + thread = test_utils.lock_a_file(a_transferred_file) + errors = project.upload_custom("rawdata", "all", "all", "all") + thread.join() + error_message = "size changed" + + # Check that errors and logs flag the transfer errors + assert errors["file_names"] == [relative_path.as_posix()] + assert error_message in errors["messages"][0] + + log = test_utils.read_log_file(project.cfg.logging_path) + + assert Path(errors["file_names"][0]).as_posix() in log + assert "Errors were detected!" in log + assert error_message in log + + # now just upload everything + project.upload_entire_project() + test_utils.delete_log_files(project.cfg.logging_path) + + # Check that it is flagged that no transfer took place for rawdata + errors = project.upload_custom("rawdata", "all", "all", "all") + + assert errors["nothing_was_transferred_rawdata"] is True + assert errors["nothing_was_transferred_derivatives"] is None + + log = test_utils.read_log_file(project.cfg.logging_path) + assert "Nothing was transferred from rawdata." in log + + test_utils.delete_log_files(project.cfg.logging_path) + + # Check that it is flagged that no transfer took place for derivatives + errors = project.upload_custom("derivatives", "all", "all", "all") + + assert errors["nothing_was_transferred_rawdata"] is None + assert errors["nothing_was_transferred_derivatives"] is True + + log = test_utils.read_log_file(project.cfg.logging_path) + assert "Nothing was transferred from derivatives." in log + + test_utils.delete_log_files(project.cfg.logging_path) + + # Check that it is flagged that no transfer took place + # for both rawdata and derivatives + errors = project.upload_entire_project() + + assert errors["nothing_was_transferred_rawdata"] is True + assert errors["nothing_was_transferred_derivatives"] is True + + log = test_utils.read_log_file(project.cfg.logging_path) + assert "Nothing was transferred from rawdata." in log + assert "Nothing was transferred from derivatives." in log diff --git a/tests/tests_transfers/local_filesystem/test_transfer.py b/tests/tests_transfers/local_filesystem/test_transfer.py index b97ab819a..039300a55 100644 --- a/tests/tests_transfers/local_filesystem/test_transfer.py +++ b/tests/tests_transfers/local_filesystem/test_transfer.py @@ -1,11 +1,9 @@ import os -import platform import re import time from pathlib import Path import pytest -from filelock import FileLock from datashuttle.configs import canonical_folders from datashuttle.configs.canonical_configs import get_broad_datatypes @@ -727,100 +725,6 @@ def monkeypatch_parse_output(top_level_folder, b): "nothing_was_transferred_derivatives": None, } - @pytest.mark.skipif( - platform.system() != "Windows", reason="Only run on windows." - ) - def test_errors_are_caught_and_logged(self, project): - """ - Create errors in the transfer by locking files, and check - the errors are correctly flagged in logs and `errors`. Also, - perform a transfer where no files are transferred, and check - this is flagged in logs and `errors`. - """ - - # Set up a folder to transfer - subs, sessions = test_utils.get_default_sub_sessions_to_test() - - test_utils.make_and_check_local_project_folders( - project, - "rawdata", - subs, - sessions, - get_broad_datatypes(), - ) - - # Lock a file then perform the transfer, causing errors. - relative_path = ( - Path("rawdata") - / subs[0] - / sessions[0] - / "ephys" - / "placeholder_file.txt" - ) - a_transferred_file = project.get_local_path() / relative_path - - test_utils.delete_log_files(project.cfg.logging_path) - - test_utils.delete_log_files(project.cfg.logging_path) - - if platform.system() == "Windows": - lock = FileLock(a_transferred_file, timeout=5) - with lock: - errors = project.upload_custom("rawdata", "all", "all", "all") - error_message = "because another process has locked " - else: - thread = test_utils.lock_a_file(a_transferred_file) - errors = project.upload_custom("rawdata", "all", "all", "all") - thread.join() - error_message = "size changed" - - # Check that errors and logs flag the transfer errors - assert errors["file_names"] == [relative_path.as_posix()] - assert error_message in errors["messages"][0] - - log = test_utils.read_log_file(project.cfg.logging_path) - - assert Path(errors["file_names"][0]).as_posix() in log - assert "Errors were detected!" in log - assert error_message in log - - # now just upload everything - project.upload_entire_project() - test_utils.delete_log_files(project.cfg.logging_path) - - # Check that it is flagged that no transfer took place for rawdata - errors = project.upload_custom("rawdata", "all", "all", "all") - - assert errors["nothing_was_transferred_rawdata"] is True - assert errors["nothing_was_transferred_derivatives"] is None - - log = test_utils.read_log_file(project.cfg.logging_path) - assert "Nothing was transferred from rawdata." in log - - test_utils.delete_log_files(project.cfg.logging_path) - - # Check that it is flagged that no transfer took place for derivatives - errors = project.upload_custom("derivatives", "all", "all", "all") - - assert errors["nothing_was_transferred_rawdata"] is None - assert errors["nothing_was_transferred_derivatives"] is True - - log = test_utils.read_log_file(project.cfg.logging_path) - assert "Nothing was transferred from derivatives." in log - - test_utils.delete_log_files(project.cfg.logging_path) - - # Check that it is flagged that no transfer took place - # for both rawdata and derivatives - errors = project.upload_entire_project() - - assert errors["nothing_was_transferred_rawdata"] is True - assert errors["nothing_was_transferred_derivatives"] is True - - log = test_utils.read_log_file(project.cfg.logging_path) - assert "Nothing was transferred from rawdata." in log - assert "Nothing was transferred from derivatives." in log - def get_paths_to_a_local_and_central_file(self, project, top_level_folder): path_to_test_file = ( Path(top_level_folder) From ad7673654b3c47e51b3dd86044522f3ef2db0fb7 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Wed, 5 Nov 2025 17:09:47 +0000 Subject: [PATCH 24/25] Fix CI script. --- .github/workflows/code_test_and_deploy.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/code_test_and_deploy.yml b/.github/workflows/code_test_and_deploy.yml index 3114c59e0..acf18edf7 100644 --- a/.github/workflows/code_test_and_deploy.yml +++ b/.github/workflows/code_test_and_deploy.yml @@ -94,7 +94,7 @@ jobs: - name: All Other Tests run: | - pytest -k test_transfers + pytest . -k test_logging # --ignore=tests/tests_transfers/ssh --ignore=tests/tests_transfers/gdrive --ignore=tests/tests_transfers/aws From 57780431c09318e12ae656c57b3e09a3d981d020 Mon Sep 17 00:00:00 2001 From: JoeZiminski Date: Wed, 5 Nov 2025 17:21:11 +0000 Subject: [PATCH 25/25] Revert ci changes. --- .github/workflows/code_test_and_deploy.yml | 49 +++++++++++----------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/.github/workflows/code_test_and_deploy.yml b/.github/workflows/code_test_and_deploy.yml index acf18edf7..950c9957c 100644 --- a/.github/workflows/code_test_and_deploy.yml +++ b/.github/workflows/code_test_and_deploy.yml @@ -68,34 +68,33 @@ jobs: # run SSH tests only on Linux because Windows and macOS # are already run within a virtual container and so cannot # run Linux containers because nested containerisation is disabled. -# - name: Test SSH (Linux only) -# if: runner.os == 'Linux' -# run: | -# sudo service mysql stop # free up port 3306 for ssh tests -# pytest tests/tests_transfers/ssh - -# - name: Test Google Drive -# env: -# GDRIVE_CLIENT_ID: ${{ secrets.GDRIVE_CLIENT_ID }} -# GDRIVE_CLIENT_SECRET: ${{ secrets.GDRIVE_CLIENT_SECRET }} -# GDRIVE_ROOT_FOLDER_ID: ${{ secrets.GDRIVE_ROOT_FOLDER_ID }} -# GDRIVE_CONFIG_TOKEN: ${{ secrets.GDRIVE_CONFIG_TOKEN }} -# run: | -# pytest tests/tests_transfers/gdrive - -# - name: Test AWS -# env: -# AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }} -# AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }} -# AWS_REGION: ${{ secrets.AWS_REGION }} -# AWS_BUCKET_NAME: ${{ secrets.AWS_BUCKET_NAME }} -# run: | -# pytest tests/tests_transfers/aws + - name: Test SSH (Linux only) + if: runner.os == 'Linux' + run: | + sudo service mysql stop # free up port 3306 for ssh tests + pytest tests/tests_transfers/ssh + + - name: Test Google Drive + env: + GDRIVE_CLIENT_ID: ${{ secrets.GDRIVE_CLIENT_ID }} + GDRIVE_CLIENT_SECRET: ${{ secrets.GDRIVE_CLIENT_SECRET }} + GDRIVE_ROOT_FOLDER_ID: ${{ secrets.GDRIVE_ROOT_FOLDER_ID }} + GDRIVE_CONFIG_TOKEN: ${{ secrets.GDRIVE_CONFIG_TOKEN }} + run: | + pytest tests/tests_transfers/gdrive + + - name: Test AWS + env: + AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }} + AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }} + AWS_REGION: ${{ secrets.AWS_REGION }} + AWS_BUCKET_NAME: ${{ secrets.AWS_BUCKET_NAME }} + run: | + pytest tests/tests_transfers/aws - name: All Other Tests run: | - pytest . -k test_logging -# --ignore=tests/tests_transfers/ssh --ignore=tests/tests_transfers/gdrive --ignore=tests/tests_transfers/aws + pytest --ignore=tests/tests_transfers/ssh --ignore=tests/tests_transfers/gdrive --ignore=tests/tests_transfers/aws build_sdist_wheels: