Skip to content

Commit

Permalink
Refs #23919 -- Replaced errno checking with PEP 3151 exceptions.
Browse files Browse the repository at this point in the history
  • Loading branch information
timgraham committed Jan 25, 2017
1 parent 3f0c4fe commit 632c4ff
Show file tree
Hide file tree
Showing 12 changed files with 55 additions and 87 deletions.
12 changes: 5 additions & 7 deletions django/contrib/sessions/backends/file.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import datetime
import errno
import logging
import os
import shutil
Expand Down Expand Up @@ -133,13 +132,12 @@ def save(self, must_create=False):
flags |= os.O_EXCL | os.O_CREAT
fd = os.open(session_file_name, flags)
os.close(fd)

except OSError as e:
if must_create and e.errno == errno.EEXIST:
raise CreateError
if not must_create and e.errno == errno.ENOENT:
except FileNotFoundError:
if not must_create:
raise UpdateError
raise
except FileExistsError:
if must_create:
raise CreateError

# Write the session file without interfering with other threads
# or processes. By writing to an atomically generated temporary
Expand Down
21 changes: 7 additions & 14 deletions django/core/cache/backends/filebased.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
"File-based cache backend"
import errno
import glob
import hashlib
import os
Expand Down Expand Up @@ -34,9 +33,8 @@ def get(self, key, default=None, version=None):
with open(fname, 'rb') as f:
if not self._is_expired(f):
return pickle.loads(zlib.decompress(f.read()))
except IOError as e:
if e.errno != errno.ENOENT:
raise
except FileNotFoundError:
pass
return default

def set(self, key, value, timeout=DEFAULT_TIMEOUT, version=None):
Expand Down Expand Up @@ -64,11 +62,9 @@ def _delete(self, fname):
return
try:
os.remove(fname)
except OSError as e:
# ENOENT can happen if the cache file is removed (by another
# process) after the os.path.exists check.
if e.errno != errno.ENOENT:
raise
except FileNotFoundError:
# The file may have been removed by another process.
pass

def has_key(self, key, version=None):
fname = self._key_to_file(key, version)
Expand Down Expand Up @@ -99,11 +95,8 @@ def _createdir(self):
if not os.path.exists(self._dir):
try:
os.makedirs(self._dir, 0o700)
except OSError as e:
if e.errno != errno.EEXIST:
raise EnvironmentError(
"Cache directory '%s' does not exist "
"and could not be created'" % self._dir)
except FileExistsError:
pass

def _key_to_file(self, key, version=None):
"""
Expand Down
4 changes: 2 additions & 2 deletions django/core/files/move.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,10 @@ def file_move_safe(old_file_name, new_file_name, chunk_size=1024 * 64, allow_ove

try:
os.remove(old_file_name)
except OSError as e:
except PermissionError as e:
# Certain operating systems (Cygwin and Windows)
# fail when deleting opened files, ignore it. (For the
# systems where this happens, temporary files will be auto-deleted
# on close anyway.)
if getattr(e, 'winerror', 0) != 32 and getattr(e, 'errno', 0) != 13:
if getattr(e, 'winerror', 0) != 32:
raise
32 changes: 13 additions & 19 deletions django/core/files/storage.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import errno
import os
from datetime import datetime
from urllib.parse import urljoin
Expand Down Expand Up @@ -224,9 +223,6 @@ def _save(self, name, content):
full_path = self.path(name)

# Create any intermediate directories that do not exist.
# Note that there is a race between os.path.exists and os.makedirs:
# if os.makedirs fails with EEXIST, the directory was created
# concurrently, and we can continue normally. Refs #16082.
directory = os.path.dirname(full_path)
if not os.path.exists(directory):
try:
Expand All @@ -240,9 +236,11 @@ def _save(self, name, content):
os.umask(old_umask)
else:
os.makedirs(directory)
except OSError as e:
if e.errno != errno.EEXIST:
raise
except FileNotFoundError:
# There's a race between os.path.exists() and os.makedirs().
# If os.makedirs() fails with FileNotFoundError, the directory
# was created concurrently.
pass
if not os.path.isdir(directory):
raise IOError("%s exists and is not a directory." % directory)

Expand Down Expand Up @@ -280,13 +278,10 @@ def _save(self, name, content):
_file.close()
else:
os.close(fd)
except OSError as e:
if e.errno == errno.EEXIST:
# Ooops, the file exists. We need a new file name.
name = self.get_available_name(name)
full_path = self.path(name)
else:
raise
except FileExistsError:
# A new name is needed if the file exists.
name = self.get_available_name(name)
full_path = self.path(name)
else:
# OK, the file save worked. Break out of the loop.
break
Expand All @@ -301,13 +296,12 @@ def delete(self, name):
assert name, "The name argument is not allowed to be empty."
name = self.path(name)
# If the file exists, delete it from the filesystem.
# If os.remove() fails with ENOENT, the file may have been removed
# concurrently, and it's safe to continue normally.
try:
os.remove(name)
except OSError as e:
if e.errno != errno.ENOENT:
raise
except FileNotFoundError:
# If os.remove() fails with FileNotFoundError, the file may have
# been removed concurrently.
pass

def exists(self, name):
return os.path.exists(self.path(name))
Expand Down
12 changes: 5 additions & 7 deletions django/core/files/uploadedfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
Classes representing uploaded files.
"""

import errno
import os
from io import BytesIO

Expand Down Expand Up @@ -71,12 +70,11 @@ def temporary_file_path(self):
def close(self):
try:
return self.file.close()
except OSError as e:
if e.errno != errno.ENOENT:
# Means the file was moved or deleted before the tempfile
# could unlink it. Still sets self.file.close_called and
# calls self.file.file.close() before the exception
raise
except FileNotFoundError:
# The file was moved or deleted before the tempfile could unlink
# it. Still sets self.file.close_called and calls
# self.file.file.close() before the exception.
pass


class InMemoryUploadedFile(UploadedFile):
Expand Down
9 changes: 3 additions & 6 deletions django/core/management/templates.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import cgi
import errno
import mimetypes
import os
import posixpath
Expand Down Expand Up @@ -76,12 +75,10 @@ def handle(self, app_or_project, name, target=None, **options):
top_dir = path.join(os.getcwd(), name)
try:
os.makedirs(top_dir)
except FileExistsError:
raise CommandError("'%s' already exists" % top_dir)
except OSError as e:
if e.errno == errno.EEXIST:
message = "'%s' already exists" % top_dir
else:
message = e
raise CommandError(message)
raise CommandError(e)
else:
top_dir = os.path.abspath(path.expanduser(target))
if not os.path.exists(top_dir):
Expand Down
15 changes: 6 additions & 9 deletions django/template/backends/dummy.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import errno
import string

from django.conf import settings
Expand Down Expand Up @@ -31,14 +30,12 @@ def get_template(self, template_name):
try:
with open(template_file, encoding=settings.FILE_CHARSET) as fp:
template_code = fp.read()
except IOError as e:
if e.errno == errno.ENOENT:
tried.append((
Origin(template_file, template_name, self),
'Source does not exist',
))
continue
raise
except FileNotFoundError:
tried.append((
Origin(template_file, template_name, self),
'Source does not exist',
))
continue

return Template(template_code)

Expand Down
8 changes: 2 additions & 6 deletions django/template/loaders/filesystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
Wrapper for loading templates from the filesystem.
"""

import errno

from django.core.exceptions import SuspiciousFileOperation
from django.template import Origin, TemplateDoesNotExist
from django.utils._os import safe_join
Expand All @@ -24,10 +22,8 @@ def get_contents(self, origin):
try:
with open(origin.name, encoding=self.engine.file_charset) as fp:
return fp.read()
except IOError as e:
if e.errno == errno.ENOENT:
raise TemplateDoesNotExist(origin)
raise
except FileNotFoundError:
raise TemplateDoesNotExist(origin)

def get_template_sources(self, template_name):
"""
Expand Down
2 changes: 1 addition & 1 deletion tests/cache/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -1360,7 +1360,7 @@ def test_get_ignores_enoent(self):
# Returns the default instead of erroring.
self.assertEqual(cache.get('foo', 'baz'), 'baz')

def test_get_does_not_ignore_non_enoent_errno_values(self):
def test_get_does_not_ignore_non_filenotfound_exceptions(self):
with mock.patch('builtins.open', side_effect=IOError):
with self.assertRaises(IOError):
cache.get('foo')
Expand Down
17 changes: 8 additions & 9 deletions tests/file_storage/tests.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import errno
import os
import shutil
import sys
Expand Down Expand Up @@ -416,9 +415,9 @@ def fake_makedirs(path):
real_makedirs(path)
elif path == os.path.join(self.temp_dir, 'raced'):
real_makedirs(path)
raise OSError(errno.EEXIST, 'simulated EEXIST')
raise FileNotFoundError()
elif path == os.path.join(self.temp_dir, 'error'):
raise OSError(errno.EACCES, 'simulated EACCES')
raise FileExistsError()
else:
self.fail('unexpected argument %r' % path)

Expand All @@ -433,8 +432,8 @@ def fake_makedirs(path):
with self.storage.open('raced/test.file') as f:
self.assertEqual(f.read(), b'saved with race')

# OSErrors aside from EEXIST are still raised.
with self.assertRaises(OSError):
# Exceptions aside from FileNotFoundError are raised.
with self.assertRaises(FileExistsError):
self.storage.save('error/test.file', ContentFile('not saved'))
finally:
os.makedirs = real_makedirs
Expand All @@ -452,9 +451,9 @@ def fake_remove(path):
real_remove(path)
elif path == os.path.join(self.temp_dir, 'raced.file'):
real_remove(path)
raise OSError(errno.ENOENT, 'simulated ENOENT')
raise FileNotFoundError()
elif path == os.path.join(self.temp_dir, 'error.file'):
raise OSError(errno.EACCES, 'simulated EACCES')
raise PermissionError()
else:
self.fail('unexpected argument %r' % path)

Expand All @@ -469,9 +468,9 @@ def fake_remove(path):
self.storage.delete('raced.file')
self.assertFalse(self.storage.exists('normal.file'))

# OSErrors aside from ENOENT are still raised.
# Exceptions aside from FileNotFoundError are raised.
self.storage.save('error.file', ContentFile('delete with error'))
with self.assertRaises(OSError):
with self.assertRaises(PermissionError):
self.storage.delete('error.file')
finally:
os.remove = real_remove
Expand Down
4 changes: 1 addition & 3 deletions tests/file_uploads/tests.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import base64
import errno
import hashlib
import json
import os
Expand Down Expand Up @@ -564,9 +563,8 @@ def test_readonly_root(self):
"""Permission errors are not swallowed"""
os.chmod(MEDIA_ROOT, 0o500)
self.addCleanup(os.chmod, MEDIA_ROOT, 0o700)
with self.assertRaises(OSError) as cm:
with self.assertRaises(PermissionError):
self.obj.testfile.save('foo.txt', SimpleUploadedFile('foo.txt', b'x'), save=False)
self.assertEqual(cm.exception.errno, errno.EACCES)

def test_not_a_directory(self):
"""The correct IOError is raised when the upload directory name exists but isn't a directory"""
Expand Down
6 changes: 2 additions & 4 deletions tests/staticfiles_tests/storage.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import errno
import os
from datetime import datetime, timedelta

Expand Down Expand Up @@ -51,9 +50,8 @@ def delete(self, name):
name = self._path(name)
try:
os.remove(name)
except OSError as e:
if e.errno != errno.ENOENT:
raise
except FileNotFoundError:
pass

def path(self, name):
raise NotImplementedError
Expand Down

0 comments on commit 632c4ff

Please sign in to comment.