[Notes] [Git][BuildStream/buildstream][bschubert/fix-atomic-move-git-repo] 2 commits: cascache.py: use move_atomic instead of manual error checking

[Benjamin Schubert <gitlab mg gitlab com>]

Title: GitLab

Benjamin Schubert pushed to branch bschubert/fix-atomic-move-git-repo at BuildStream / buildstream

Commits:

• bf5a7a2f
  by Benjamin Schubert at 2018-11-09T14:43:04Z

  cascache.py: use move_atomic instead of manual error checking
  

• d68daa02
  by Benjamin Schubert at 2018-11-09T14:43:04Z

  sources/pip.py: Use move_atomic instead of manual os.rename
  
  This uses move_atomic insteand of the manual os.rename and manual error
  checking and throws a SourceError for consistency with other modules.
  

2 changed files:

• buildstream/_artifactcache/cascache.py
• buildstream/plugins/sources/pip.py

Changes:

buildstream/_artifactcache/cascache.py

---

...	...	@@ -24,7 +24,6 @@ import os
24	24	import stat
25	25	import tempfile
26	26	import uuid
27		-import errno
28	27	from urllib.parse import urlparse
29	28
30	29	import grpc
...	...	@@ -108,17 +107,13 @@ class CASCache():
108	107	checkoutdir = os.path.join(tmpdir, ref)
109	108	self._checkout(checkoutdir, tree)
110	109
111		- os.makedirs(os.path.dirname(dest), exist_ok=True)
112	110	try:
113		- os.rename(checkoutdir, dest)
	111	+ utils.move_atomic(checkoutdir, dest)
	112	+ except utils.DirectoryExistsError:
	113	+ # Another process beat us to rename
	114	+ pass
114	115	except OSError as e:
115		- # With rename it's possible to get either ENOTEMPTY or EEXIST
116		- # in the case that the destination path is a not empty directory.
117		- #
118		- # If rename fails with these errors, another process beat
119		- # us to it so just ignore.
120		- if e.errno not in [errno.ENOTEMPTY, errno.EEXIST]:
121		- raise CASError("Failed to extract directory for ref '{}': {}".format(ref, e)) from e
	116	+ raise CASError("Failed to extract directory for ref '{}': {}".format(ref, e)) from e
122	117
123	118	return dest
124	119

buildstream/plugins/sources/pip.py

---

...	...	@@ -68,7 +68,6 @@ details on common configuration options for sources.
68	68	The ``pip`` plugin is available since :ref:`format version 16 <project_format_version>`
69	69	"""
70	70
71		-import errno
72	71	import hashlib
73	72	import os
74	73	import re
...	...	@@ -192,13 +191,14 @@ class PipSource(Source):
192	191	# process has fetched the sources before us and ensure that we do
193	192	# not raise an error in that case.
194	193	try:
195		- os.makedirs(self._mirror)
196		- os.rename(package_dir, self._mirror)
197		- except FileExistsError:
198		- return
	194	+ utils.move_atomic(package_dir, self._mirror)
	195	+ except utils.DirectoryExistsError:
	196	+ # Another process has beaten us and has fetched the sources
	197	+ # before us.
	198	+ pass
199	199	except OSError as e:
200		- if e.errno != errno.ENOTEMPTY:
201		- raise
	200	+ raise SourceError("{}: Failed to move downloaded pip packages from '{}' to '{}': {}"
	201	+ .format(self package_dir, self._mirror, e)) from e
202	202
203	203	def stage(self, directory):
204	204	with self.timed_activity("Staging Python packages", silent_nested=True):

—
View it on GitLab.
You're receiving this email because of your account on gitlab.com. If you'd like to receive fewer emails, you can adjust your notification settings.