This is an archive of the discontinued Mercurial Phabricator instance.

Differential D7957

lfs: explicitly close the file handle for the blob being uploaded
ClosedPublic

Authored by mharbison72 on Jan 21 2020, 1:37 PM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHG2ad4e8aefcf4: lfs: explicitly close the file handle for the blob being uploaded
rHGfad1db080e99: lfs: explicitly close the file handle for the blob being uploaded
Summary

The previous code relied on reading the blob fully to close it. The obvious
problem is if an error occurs before that point. But there is also a problem
when using workers where the data may need to be re-read, which can't happen
once it is closed. This eliminates the surprising behavior before attempting to
fix the worker problem.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Revision Contents
Changeset List

PathPackages
Mhgext/lfs/blobstore.py (10 lines)

Diff 19507

hgext/lfs/blobstore.py

fp.seek(0) fp.seek(0)
def __len__(self): def __len__(self):
return self._len return self._len
def read(self, size): def read(self, size):
if self._fp is None: if self._fp is None:
return b'' return b''
data = self._fp.read(size) return self._fp.read(size)
if not data:
def close(self):
if self._fp is not None:
self._fp.close() self._fp.close()
self._fp = None self._fp = None
return data
class local(object): class local(object):
"""Local blobstore for large file contents. """Local blobstore for large file contents.
This blobstore is used both as a cache and as a staging area for large blobs This blobstore is used both as a cache and as a staging area for large blobs
to be uploaded to the remote blobstore. to be uploaded to the remote blobstore.
""" """
) )
except util.urlerr.urlerror as ex: except util.urlerr.urlerror as ex:
hint = _(b'attempted connection to %s') % pycompat.bytesurl( hint = _(b'attempted connection to %s') % pycompat.bytesurl(
util.urllibcompat.getfullurl(request) util.urllibcompat.getfullurl(request)
) )
raise LfsRemoteError( raise LfsRemoteError(
_(b'LFS error: %s') % _urlerrorreason(ex), hint=hint _(b'LFS error: %s') % _urlerrorreason(ex), hint=hint
) )
finally:
if request.data:
request.data.close()
def _batch(self, pointers, localstore, action): def _batch(self, pointers, localstore, action):
if action not in [b'upload', b'download']: if action not in [b'upload', b'download']:
raise error.ProgrammingError(b'invalid Git-LFS action: %s' % action) raise error.ProgrammingError(b'invalid Git-LFS action: %s' % action)
response = self._batchrequest(pointers, action) response = self._batchrequest(pointers, action)
objects = self._extractobjects(response, pointers, action) objects = self._extractobjects(response, pointers, action)
total = sum(x.get(b'size', 0) for x in objects) total = sum(x.get(b'size', 0) for x in objects)