From f07a613b982c0e2efa5bd3e42a3b5abed63ae8d2 Mon Sep 17 00:00:00 2001
From: Christian Monch <christian.moench@web.de>
Date: Thu, 13 Jun 2024 14:38:27 +0200
Subject: [PATCH] feat(atomic): describe atomicity property

This commit adds a description of the atomicity
properties of upload to the doc-string of the method
`url_operations.base.UrlOperations.upload`.
---
 datalad_next/url_operations/base.py            | 5 +++++
 datalad_next/url_operations/tests/test_file.py | 9 ++++-----
 datalad_next/url_operations/tests/test_ssh.py  | 7 +++----
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/datalad_next/url_operations/base.py b/datalad_next/url_operations/base.py
index 2c16c8ca..ea13ede0 100644
--- a/datalad_next/url_operations/base.py
+++ b/datalad_next/url_operations/base.py
@@ -179,6 +179,11 @@ def upload(self,
                timeout: float | None = None) -> Dict:
         """Upload from a local file or stream to a URL
 
+        Whenever possible, uploads are performed atomically This means that the
+        destination will never see a partially uploaded file. It will either
+        see the previous content (or nothing) or the newly uploaded content.
+        Note: this is not supported by all implementations of URL-operations.
+
         Parameters
         ----------
         from_path: Path or None
diff --git a/datalad_next/url_operations/tests/test_file.py b/datalad_next/url_operations/tests/test_file.py
index d52cf44f..62e7dbe6 100644
--- a/datalad_next/url_operations/tests/test_file.py
+++ b/datalad_next/url_operations/tests/test_file.py
@@ -38,8 +38,7 @@ def test_file_url_download(tmp_path):
         ops.download(test_url, download_path)
 
 
-@pytest.mark.parametrize('atomic', [True, False])
-def test_file_url_upload(tmp_path, monkeypatch, atomic):
+def test_file_url_upload(tmp_path, monkeypatch):
     payload = 'payload'
     payload_file = tmp_path / 'payload'
     test_upload_path = tmp_path / 'myfile'
@@ -48,13 +47,13 @@ def test_file_url_upload(tmp_path, monkeypatch, atomic):
     # missing source file
     # standard exception, makes no sense to go custom thinks mih
     with pytest.raises(FileNotFoundError):
-        ops.upload(payload_file, test_upload_url, atomic=atomic)
+        ops.upload(payload_file, test_upload_url)
     # no empty targets lying around
     assert not test_upload_path.exists()
 
     # now again
     payload_file.write_text(payload)
-    props = ops.upload(payload_file, test_upload_url, hash=['md5'], atomic=atomic)
+    props = ops.upload(payload_file, test_upload_url, hash=['md5'])
     assert test_upload_path.read_text() == 'payload'
     assert props['content-length'] == len(payload)
     assert props['md5'] == '321c3cf486ed509164edec1e1981fec8'
@@ -65,7 +64,7 @@ def test_file_url_upload(tmp_path, monkeypatch, atomic):
         m.setattr(sys, 'stdin',
                   io.TextIOWrapper(io.BytesIO(
                       bytes(payload, encoding='utf-8'))))
-        props = ops.upload(None, from_stdin_url, hash=['md5'], atomic=atomic)
+        props = ops.upload(None, from_stdin_url, hash=['md5'])
         assert props['md5'] == '321c3cf486ed509164edec1e1981fec8'
         assert props['content-length'] == len(payload)
 
diff --git a/datalad_next/url_operations/tests/test_ssh.py b/datalad_next/url_operations/tests/test_ssh.py
index d0f3cb69..680dafd6 100644
--- a/datalad_next/url_operations/tests/test_ssh.py
+++ b/datalad_next/url_operations/tests/test_ssh.py
@@ -49,8 +49,7 @@ def test_ssh_url_download(tmp_path, monkeypatch, sshserver):
 
 # path magic inside the test is posix only
 @skip_if_on_windows
-@pytest.mark.parametrize('atomic', [True, False])
-def test_ssh_url_upload(tmp_path, monkeypatch, sshserver, atomic):
+def test_ssh_url_upload(tmp_path, monkeypatch, sshserver):
     ssh_url, ssh_localpath = sshserver
     payload = 'surprise!'
     payload_path = tmp_path / 'payload'
@@ -60,7 +59,7 @@ def test_ssh_url_upload(tmp_path, monkeypatch, sshserver, atomic):
 
     # standard error if local source is not around
     with pytest.raises(FileNotFoundError):
-        ops.upload(payload_path, upload_url, atomic=atomic)
+        ops.upload(payload_path, upload_url)
 
     payload_path.write_text(payload)
     # upload creates parent dirs, so the next just works.
@@ -69,7 +68,7 @@ def test_ssh_url_upload(tmp_path, monkeypatch, sshserver, atomic):
     # server-side preconditions first.
     # this functionality is not about exposing a full
     # remote FS abstraction -- just upload
-    ops.upload(payload_path, upload_url, atomic=atomic)
+    ops.upload(payload_path, upload_url)
     assert upload_path.read_text() == payload