Apply some fixes from self review 1.

Author: JoeZiminski

datashuttle/configs/canonical_folders.py

@@ -82,7 +82,7 @@ def get_datashuttle_path() -> Path:
     return Path.home() / ".datashuttle"
 
 
-def get_internal_datashuttle_from_path():
+def get_internal_datashuttle_from_path() -> Path:
     """Get a placeholder path for `validate_project_from_path()`."""
     return get_datashuttle_path() / "_datashuttle_from_path"
 
@@ -100,16 +100,19 @@ def get_project_datashuttle_path(project_name: str) -> Tuple[Path, Path]:
     return base_path, temp_logs_path
 
 
-def get_rclone_config_base_path():
+def get_rclone_config_base_path() -> Path:
     """Return the path to the Rclone config file.
 
     This is used for RClone config files for transfer targets (ssh, aws, gdrive).
     This should match where RClone itself stores the config by default,
     as described here: https://rclone.org/docs/#config-string
 
-    Because RClone's resolution is a little complex, in some rare cases the
-    below may not match where RClone stores its configs. This just means that
-    local filesystem configs and transfer configs are stored in a separate place,
+    Because RClone's resolution process for where it stores its config files
+    is a little complex, in some rare cases the below may not match where
+    RClone stores its configs. This just means that local filesystem configs,
+    which are stored in the default `rclone.conf` file for backwards compatibility
+    reasons. and transfer configs which are stored in their own file at the
+    path returned from this function, are stored in a separate places,
     which is not a huge deal.
     """
     if platform.system() == "Windows":

datashuttle/configs/config_class.py

@@ -37,8 +37,8 @@ def __init__(
     ) -> None:
         """Initialize the Configs class with project name, file path, and config dictionary.
 
-        This class also holds `RCloneConfigs` under the `.rclone` attribute to manage
-        the configs
+        This class also holds `RCloneConfigs` that manage the Rclone config files
+        used for transfer.
 
         Parameters
         ----------

datashuttle/configs/rclone_configs.py

@@ -1,42 +1,44 @@
 from __future__ import annotations
 
-from typing import TYPE_CHECKING, Dict, Optional
+from typing import TYPE_CHECKING, Optional
 
 if TYPE_CHECKING:
     from pathlib import Path
 
-    from datashuttle.utils.custom_types import (
-        OverwriteExistingFiles,
-    )
+    from datashuttle.configs.configs_class import Configs
 
 import yaml
 
 from datashuttle.configs import canonical_folders
-from datashuttle.utils import rclone_encryption, utils
+from datashuttle.utils import rclone_encryption
 
 
 class RCloneConfigs:
     """Class to manage the RClone configuration file.
 
     This is a file that RClone creates to hold all information about local and
-    remote transfer targets. For example, the ssh RClone config holds the private key.
+    central transfer targets. For example, the SSH RClone config holds the private key,
+    the GDrive rclone config holds the access token, etc.
 
     In datashuttle, local filesystem configs uses the Rclone default configuration file,
-    that RClone manages. However, remote transfers to ssh, aws and gdrive are held in
-    separate config files (set using RClone's --config argument). Then being separate
-    means these files can be separately encrypted.
+    that RClone manages, for backwards comatability reasons. However, SSH, AWS and GDrive
+    configs are stored in separate config files (set using RClone's --config argument).
+    Then being separate means these files can be separately encrypted.
 
     This class tracks the state on whether a RClone config is encrypted, as well
     as provides the default names for the rclone conf (e.g. central_<project_name>_<connection_method>).
 
     Parameters
     ----------
+    datashuttle_configs
+        Parent Configs class.
+
     config_base_class
         Path to the datashuttle configs folder where all configs for the project are stored.
 
     """
 
-    def __init__(self, datashuttle_configs, config_base_path):
+    def __init__(self, datashuttle_configs: Configs, config_base_path: Path):
         """Construct the class."""
         self.datashuttle_configs = datashuttle_configs
         self.rclone_encryption_state_file_path = (
@@ -73,8 +75,9 @@ def load_rclone_config_is_encrypted(self) -> dict:
     def set_rclone_config_encryption_state(self, value: bool) -> None:
         """Store the current state of the rclone config encryption for the `connection_method`.
 
-        Note that this is stored to disk each call (rather than tracked locally) to ensure
-        it is updated live if updated through the Python API while the TUI is also running.
+        Note that this is stored to disk each call (rather than tracked in memory)
+        to ensure it is updated properly if changed through the Python API
+        while the TUI is also running.
         """
         assert rclone_encryption.connection_method_requires_encryption(
             self.datashuttle_configs["connection_method"]
@@ -119,27 +122,6 @@ def get_rclone_central_connection_config_filepath(self) -> Path:
             / f"{self.get_rclone_config_name()}.conf"
         )
 
-    def make_rclone_transfer_options(
-        self, overwrite_existing_files: OverwriteExistingFiles, dry_run: bool
-    ) -> Dict:
-        """Create a dictionary of rclone transfer options."""
-        allowed_overwrite = ["never", "always", "if_source_newer"]
-
-        if overwrite_existing_files not in allowed_overwrite:
-            utils.log_and_raise_error(
-                f"`overwrite_existing_files` not "
-                f"recognised, must be one of: "
-                f"{allowed_overwrite}",
-                ValueError,
-            )
-
-        return {
-            "overwrite_existing_files": overwrite_existing_files,
-            "show_transfer_progress": True,
-            "transfer_verbosity": "vv",
-            "dry_run": dry_run,
-        }
-
     def delete_existing_rclone_config_file(self) -> None:
         """Delete the Rclone config file if it exists."""
         rclone_config_filepath = (

datashuttle/datashuttle_class.py

@@ -790,7 +790,7 @@ def _transfer_specific_file_or_folder(
             upload_or_download,
             top_level_folder,
             include_list,
-            self.cfg.rclone.make_rclone_transfer_options(
+            rclone.make_rclone_transfer_options(
                 overwrite_existing_files, dry_run
             ),
         )

datashuttle/utils/data_transfer.py

@@ -99,7 +99,7 @@ def __init__(
                 self.__upload_or_download,
                 self.__top_level_folder,
                 include_list,
-                cfg.rclone.make_rclone_transfer_options(
+                rclone.make_rclone_transfer_options(
                     overwrite_existing_files, dry_run
                 ),
             )

datashuttle/utils/rclone.py

@@ -7,7 +7,10 @@
     from subprocess import CompletedProcess
 
     from datashuttle.configs.config_class import Configs
-    from datashuttle.utils.custom_types import TopLevelFolder
+    from datashuttle.utils.custom_types import (
+        OverwriteExistingFiles,
+        TopLevelFolder,
+    )
 
 import json
 import os
@@ -635,7 +638,7 @@ def transfer_data(
 
     rclone_options
         A list of options to pass to Rclone's copy function.
-        see `cfg.rclone.make_rclone_transfer_options()`.
+        see `make_rclone_transfer_options()`.
 
     Returns
     -------
@@ -677,6 +680,28 @@ def transfer_data(
     return output
 
 
+def make_rclone_transfer_options(
+    overwrite_existing_files: OverwriteExistingFiles, dry_run: bool
+) -> Dict:
+    """Create a dictionary of rclone transfer options."""
+    allowed_overwrite = ["never", "always", "if_source_newer"]
+
+    if overwrite_existing_files not in allowed_overwrite:
+        utils.log_and_raise_error(
+            f"`overwrite_existing_files` not "
+            f"recognised, must be one of: "
+            f"{allowed_overwrite}",
+            ValueError,
+        )
+
+    return {
+        "overwrite_existing_files": overwrite_existing_files,
+        "show_transfer_progress": True,
+        "transfer_verbosity": "vv",
+        "dry_run": dry_run,
+    }
+
+
 def get_local_and_central_file_differences(
     cfg: Configs,
     top_level_folders_to_check: List[TopLevelFolder],