Repository: SciCatProject/scitacean

Issue:
Do not require matching files size in download / upload
File sizes can vary depending on the block size of the filesystem. With the current setup at ESS, the sizes in datasets are almost always different from the physical file sizes.
So we should not raise an exception if there is a mismatch but simply log a message.

Discussion:

Patch:
diff --git a/src/scitacean/file.py b/src/scitacean/file.py
index 26e0cc1..8ce3dd0 100644
--- a/src/scitacean/file.py
+++ b/src/scitacean/file.py
@@ -255,6 +255,7 @@ class File:
         remote_gid: Optional[str] = None,
         remote_perm: Optional[str] = None,
         remote_creation_time: Optional[datetime] = None,
+        remote_size: Optional[int] = None,
     ) -> File:
         """Return new file metadata after an upload.
 
@@ -274,6 +275,8 @@ class File:
         remote_creation_time:
             Time the file became available on remote.
             Defaults to the current time in UTC.
+        remote_size:
+            File size on remote.
 
         Returns
         -------
@@ -291,12 +294,12 @@ class File:
         )
         return dataclasses.replace(
             self,
-            _remote_size=self.size,
+            _remote_size=remote_size if remote_size is not None else self.size,
             _remote_checksum=self.checksum(),
             **{key: val for key, val in args.items() if val is not None},
         )
 
-    def downloaded(self, *, local_path: Path) -> File:
+    def downloaded(self, *, local_path: Union[str, Path]) -> File:
         """Return new file metadata after a download.
 
         Assumes that the input file exists on remote.
@@ -313,7 +316,7 @@ class File:
             A new file object.
         """
         return dataclasses.replace(
-            self, local_path=local_path, _checksum_cache=_Checksum()
+            self, local_path=Path(local_path), _checksum_cache=_Checksum()
         )
 
     def validate_after_download(self):
@@ -357,10 +360,14 @@ class File:
     def _validate_after_download_file_size(self):
         actual = file_size(self.local_path)
         if actual != self._remote_size:
-            _log_and_raise(
-                IntegrityError,
-                f"Size of file '{self.local_path}' ({actual}B) does not "
-                f"match size stored in dataset ({self._remote_size}B)",
+            get_logger().info(
+                "Size of downloaded file '%s' (%d bytes) does not "
+                "match size reported in dataset (%d bytes)."
+                "This may be due to a difference in file systems and perfectly fine. "
+                "Or it is caused by an error during download.",
+                self.local_path,
+                actual,
+                self._remote_size,
             )
 
 
diff --git a/src/scitacean/logging.py b/src/scitacean/logging.py
index 80aaf33..3fdf1da 100644
--- a/src/scitacean/logging.py
+++ b/src/scitacean/logging.py
@@ -1,8 +1,20 @@
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright (c) 2023 SciCat Project (https://github.com/SciCatProject/scitacean)
+"""Utilities for logging in Scitacean.
+
+The object returned by :func:`scitacean.get_logger` is the only logger
+used by Scitacean. Scitacean does not configure it in any way.
+You are free to do so.
+"""
 
 import logging
 
 
-def get_logger():
-    return logging.getLogger("scitacean")
+def logger_name() -> str:
+    """Return the name of Scitacean's logger."""
+    return "scitacean"
+
+
+def get_logger() -> logging.Logger:
+    """Return the logger used by Scitacean."""
+    return logging.getLogger(logger_name())
diff --git a/src/scitacean/transfer/ssh.py b/src/scitacean/transfer/ssh.py
index 985edf4..9480aab 100644
--- a/src/scitacean/transfer/ssh.py
+++ b/src/scitacean/transfer/ssh.py
@@ -139,16 +139,10 @@ class SSHUploadConnection:
             remote_uid=st.st_uid,
             remote_creation_time=st.st_mtime,
             remote_perm=st.st_mode,
+            remote_size=st.st_size,
         )
 
     def _validate_upload(self, file: File, st: SFTPAttributes):
-        if file.size != st.st_size:
-            raise FileUploadError(
-                f"Upload of file {file.remote_path} failed: "
-                f"Size of uploaded file ({st.st_size}) does not "
-                f"match size of local file ({file.size})"
-            )
-
         if (checksum := self._compute_checksum(file)) is None:
             return
         if checksum != file.checksum():


Test patch:
diff --git a/tests/common/backend.py b/tests/common/backend.py
index 2715544..99a7d71 100644
--- a/tests/common/backend.py
+++ b/tests/common/backend.py
@@ -92,7 +92,9 @@ def _merge_seed_file(
 
 def configure(target_dir: Path) -> Path:
     target = Path(target_dir) / "scicatlive"
-    shutil.copytree(_SCICAT_DOCKER_CONFIG.parent, target)
+    shutil.copytree(
+        _SCICAT_DOCKER_CONFIG.parent, target, ignore=shutil.ignore_patterns(".git")
+    )
     _merge_seed_file(target, _SCICAT_DATASET_SEED_FILE, load_datasets())
     _merge_seed_file(target, _SCICAT_ORIG_DATABLOCK_SEED_FILE, load_orig_datablocks())
 
diff --git a/tests/download_test.py b/tests/download_test.py
index c04ba9b..9279eb6 100644
--- a/tests/download_test.py
+++ b/tests/download_test.py
@@ -10,6 +10,7 @@ from dateutil.parser import parse as parse_date
 
 from scitacean import PID, Client, Dataset, DatasetType, File, IntegrityError
 from scitacean.filesystem import RemotePath
+from scitacean.logging import logger_name
 from scitacean.model import DataFile, OrigDatablock, RawDataset
 from scitacean.testing.transfer import FakeFileTransfer
 
@@ -226,7 +227,7 @@ def test_download_files_detects_bad_checksum(fs, dataset_and_files):
         client.download_files(dataset, target="./download", select="file.txt")
 
 
-def test_download_files_detects_bad_size(fs, dataset_and_files):
+def test_download_files_detects_bad_size(fs, dataset_and_files, caplog):
     dataset, contents = dataset_and_files
 
     content = b"random-stuff"
@@ -246,5 +247,7 @@ def test_download_files_detects_bad_size(fs, dataset_and_files):
             fs=fs, files={dataset.source_folder / "file.txt": content}
         ),
     )
-    with pytest.raises(IntegrityError):
+    with caplog.at_level("INFO", logger=logger_name()):
         client.download_files(dataset, target="./download", select="file.txt")
+    assert "does not match size reported in dataset" in caplog.text
+    assert "89412" in caplog.text
diff --git a/tests/file_test.py b/tests/file_test.py
index e77c816..ca745cc 100644
--- a/tests/file_test.py
+++ b/tests/file_test.py
@@ -3,12 +3,14 @@
 import hashlib
 from dataclasses import replace
 from datetime import datetime, timedelta, timezone
+from pathlib import Path
 
 import pytest
 from dateutil.parser import parse as parse_date
 
-from scitacean import File, IntegrityError
+from scitacean import File, IntegrityError, RemotePath
 from scitacean.file import checksum_of_file
+from scitacean.logging import logger_name
 from scitacean.model import DataFile
 
 from .common.files import make_file
@@ -165,8 +167,8 @@ def test_downloaded():
     assert downloaded.is_on_local
     assert downloaded.is_on_remote
 
-    assert downloaded.remote_path == "dir/stream.s"
-    assert downloaded.local_path == "/local/stream.s"
+    assert downloaded.remote_path == RemotePath("dir/stream.s")
+    assert downloaded.local_path == Path("/local/stream.s")
     assert downloaded.remote_perm == "xrw"
     assert downloaded.created_by == "creator-id"
 
@@ -269,8 +271,7 @@ def test_validate_after_download_ignores_checksum_if_no_algorithm(fake_file):
     downloaded.validate_after_download()
 
 
-@pytest.mark.parametrize("checksum_algorithm", ("md5", None))
-def test_validate_after_download_detects_size_mismatch(fake_file, checksum_algorithm):
+def test_validate_after_download_detects_size_mismatch(fake_file, caplog):
     model = DataFile(
         path=fake_file["path"].name,
         size=fake_file["size"] + 100,
@@ -279,9 +280,9 @@ def test_validate_after_download_detects_size_mismatch(fake_file, checksum_algor
     )
     file = replace(
         File.from_scicat(model),
-        checksum_algorithm=checksum_algorithm,
+        checksum_algorithm=None,
     )
     downloaded = file.downloaded(local_path=fake_file["path"])
-
-    with pytest.raises(IntegrityError):
+    with caplog.at_level("INFO", logger=logger_name()):
         downloaded.validate_after_download()
+    assert "does not match size reported in dataset" in caplog.text