Merge pull request #89 from wimglenn/v1.2.2

slight cleanup, bump version for release

aocd/models.py

@@ -219,7 +219,7 @@ def input_data(self):
             log.error(response.text)
             raise AocdError("Unexpected response")
         data = response.text
-        log.info('saving the puzzle input token=%s', sanitized)
+        log.info("saving the puzzle input token=%s", sanitized)
         atomic_write_file(self.input_data_fname, data)
         return data.rstrip("\r\n")
 

aocd/utils.py

@@ -4,6 +4,7 @@
 import logging
 import os
 import requests
+import shutil
 import sys
 import tempfile
 import time
@@ -112,26 +113,18 @@ def get_owner(token):
     result = ".".join([auth_source, username, userid])
     return result
 
+
 def atomic_write_file(fname, contents_str):
     """Atomically write a string to a file by writing it to a temporary file, and then
     renaming it to the final destination name. This solves a race condition where existence
-    of a file doesn't necessarily mean the contents are all correct yet.
-
-    Note that the behavior of writing *different* files to the same filename is undefined, as
-    there's no portable 'rename and replace' behavior in Python 2.7."""
+    of a file doesn't necessarily mean the contents are all correct yet."""
     _ensure_intermediate_dirs(fname)
-    with tempfile.NamedTemporaryFile(mode='w', dir=os.path.dirname(fname), delete=False) as f:
-        tmp_name = f.name
-        log.info('writing to %s', tmp_name)
+    with tempfile.NamedTemporaryFile(mode="w", dir=os.path.dirname(fname), delete=False) as f:
+        log.debug("writing to tempfile @ %s", f.name)
         f.write(contents_str)
-    try:
-        os.rename(tmp_name, fname)
-    except OSError as e:
-        if e.errno == errno.EEXIST:
-            log.warning('cannot rename %s (temporary) to %s as it already exists; deleting the temporary', tmp_name, fname)
-            os.unlink(tmp_name)
-        else:
-            raise
+    log.debug("moving %s -> %s", f.name, fname)
+    shutil.move(f.name, fname)
+
 
 def _cli_guess(choice, choices):
     if choice in choices:

aocd/version.py

@@ -1 +1 @@
-__version__ = "1.2.1"
+__version__ = "1.2.2"

tests/test_get_data.py

@@ -1,11 +1,11 @@
-import logging
-
-import pytest
 import io
+import logging
 import os
 import sys
 import threading
 
+import pytest
+
 import aocd
 from aocd.exceptions import AocdError
 from aocd.exceptions import PuzzleLockedError
@@ -46,6 +46,7 @@ def test_saved_data_is_reused_if_available(aocd_data_dir, requests_mock):
     assert data == "saved data for year 2018 day 1"
     assert not mock.called
 
+
 def test_server_error(requests_mock, caplog):
     mock = requests_mock.get(
         url="https://adventofcode.com/2101/day/1/input",
@@ -123,6 +124,7 @@ def test_corrupted_cache(aocd_data_dir):
     with pytest.raises(IOError):
         aocd.get_data(year=2018, day=1)
 
+
 def test_race_on_download_data(mocker, aocd_data_dir, requests_mock):
     requests_mock.get(
         url="https://adventofcode.com/2018/day/1/input",
@@ -137,7 +139,7 @@ def logging_os_open(path, *args, **kwargs):
         ret = real_os_open(path, *args, **kwargs)
         fds[ret] = path
         return ret
-    mocker.patch('os.open', side_effect=logging_os_open)
+    mocker.patch("os.open", side_effect=logging_os_open)
 
     # This doesn't use unittest.mock_open because we actually do want the faked object to be functional.
     # We don't want fake results or to assert things are called in a certain order; we just want the
@@ -146,7 +148,7 @@ def generate_open(real_open):
         def open_impl(file, *args, **kwargs):
             res = real_open(file, *args, **kwargs)
             filename = fds[file] if isinstance(file, int) else file
-            if 'aocd-data' not in filename:
+            if "aocd-data" not in filename:
                 return res
             open_evt.set()
             real_write = res.write
@@ -156,11 +158,11 @@ def write(*args, **kwargs):
             res.write = write
             return res
         return open_impl
-    mocker.patch('io.open', side_effect=generate_open(io.open))
+    mocker.patch("io.open", side_effect=generate_open(io.open))
     PY2 = sys.version_info.major < 3
-    mocker.patch('__builtin__.open' if PY2 else 'builtins.open', side_effect=generate_open(open))
+    mocker.patch("__builtin__.open" if PY2 else "builtins.open", side_effect=generate_open(open))
 
-    t = threading.Thread(target=aocd.get_data, kwargs={'year': 2018, 'day': 1})
+    t = threading.Thread(target=aocd.get_data, kwargs={"year": 2018, "day": 1})
     t.start()
     # This doesn't quite work on python 2 because the io.open patch doesn't seem to work.
     # We still get coverage by making sure the right thing happens in py3, though.

tests/test_models.py

@@ -362,6 +362,7 @@ def test_user_from_unknown_id(aocd_config_dir):
     with pytest.raises(UnknownUserError("User with id 'blah' is not known")):
         User.from_id("blah")
 
+
 def test_example_data_cache(aocd_data_dir, requests_mock):
     mock = requests_mock.get(
         url="https://adventofcode.com/2018/day/1",
@@ -376,4 +377,3 @@ def test_example_data_cache(aocd_data_dir, requests_mock):
     requests_mock.reset()
     assert puzzle.example_data == "1\n2\n3"
     assert not mock.called
-

tests/test_utils.py

@@ -1,11 +1,13 @@
 import platform
 import sys
+
 import pytest
+from freezegun import freeze_time
+
 from aocd.exceptions import DeadTokenError
 from aocd.utils import atomic_write_file
 from aocd.utils import blocker
 from aocd.utils import get_owner
-from freezegun import freeze_time
 
 
 cpython = platform.python_implementation() == "CPython"
@@ -64,11 +66,11 @@ def test_get_owner_google(requests_mock):
     owner = get_owner("...")
     assert owner == "google.wim.1"
 
+
 def test_atomic_write_file(aocd_data_dir):
     target = aocd_data_dir / "foo/bar/baz.txt"
     # Python 2.7 requires inputs to os.path.expanduser to be strings, not PosixPath (which is missing startswith)
-    atomic_write_file(str(target), "9cdfc92d-c0d2-4af7-87e2-06e49619b30e")
-    assert target.read_text() == "9cdfc92d-c0d2-4af7-87e2-06e49619b30e"
-    atomic_write_file(str(target), "91b80012-3cee-4cbe-8242-460a081c9d73")
-    # It's actually OS-dependent which file wins. But we want to make sure we have coverage
-    # of the unhappy path.
+    atomic_write_file(str(target), "123")  # no clobber
+    assert target.read_text() == "123"
+    atomic_write_file(str(target), "456")  # clobber existing
+    assert target.read_text() == "456"