From: Michael Orlitzky <michael@orlitzky.com>
Date: Fri, 4 Apr 2025 01:45:39 +0000 (-0400)
Subject: src/geoipyupdate/__init__.py: try os.replace() before shutil.move()
X-Git-Tag: 0.0.4~1
X-Git-Url: http://gitweb.michael.orlitzky.com/?a=commitdiff_plain;h=8ed9904df585e46878f891c746c30bfa5c54fed0;p=geoipyupdate.git

src/geoipyupdate/__init__.py: try os.replace() before shutil.move()

It turns out that shutil.move() is not atomic, which can lead to
issues when replacing the database under heavy load. There is an
alternative in os.replace() that *is* atomic, but does not work across
filesystems. For the best of both worlds, we now try os.replace(), and
fall back to shutil.move() if it fails.

Testing shows that os.replace() raises an OSError across multiple
filesystems, so that's what we catch. The documentation doesn't
specify one way or the other.

Thanks to Paulo Magalhães for report.
---

diff --git a/src/geoipyupdate/__init__.py b/src/geoipyupdate/__init__.py
index ace4bdc..a9e3bf5 100644
--- a/src/geoipyupdate/__init__.py
+++ b/src/geoipyupdate/__init__.py
@@ -131,5 +131,12 @@ def main():
         # minus whatever the user has set in his umask.
         os.chmod(g.name, 0o664 & ~old_umask)
 
-        # Overwrite the old database file with the new (gunzipped) one.
-        shutil.move(g.name, dbfile)
+        # Overwrite the old database file with the new (gunzipped)
+        # one. The os.replace() method is preferable because it is
+        # atomic, but it will raise an OSError if the source and
+        # destination are on different filesystems. We fall back
+        # to shutil.move in that case.
+        try:
+            os.replace(g.name, dbfile)
+        except OSError:
+            shutil.move(g.name, dbfile)