quark created this revision. Herald added a subscriber: mercurial-devel. Herald added a reviewer: hg-reviewers.
REVISION SUMMARY `test-lfs-test-server.t` refers to a FB internal domain and requires certain implementation (ex. set error code to 404) at that endpoint. Without any workaround, It should in theory error out like "Domain cannot be resolved". I don't know how Matt Harbison ran the test. This patch changes the test to only depend on `lfs-test-server`. Unfortunately the logic has to be changed since `lfs-test-server` does not set error code to 404 but just removes "download" from "actions". REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1849 AFFECTED FILES hgext/lfs/blobstore.py tests/test-lfs-test-server.t CHANGE DETAILS diff --git a/tests/test-lfs-test-server.t b/tests/test-lfs-test-server.t --- a/tests/test-lfs-test-server.t +++ b/tests/test-lfs-test-server.t @@ -160,12 +160,12 @@ $ rm -rf .hg/store/lfs $ rm -rf `hg config lfs.usercache` $ hg update -C '.^' - abort: LFS server claims required objects do not exist: - 8e6ea5f6c066b44a0efa43bcce86aea73f17e6e23f0663df0251e7524e140a13! + abort: LFS server error. Remote object for "b" not found:(.*)! (re) [255] Check error message when object does not exist: + $ cd $TESTTMP $ hg init test && cd test $ echo "[extensions]" >> .hg/hgrc $ echo "lfs=" >> .hg/hgrc @@ -183,7 +183,22 @@ x-is-binary 0 $ cd .. $ rm -rf `hg config lfs.usercache` - $ hg --config 'lfs.url=https://dewey-lfs.vip.facebook.com/lfs' clone test test2 + +(Restart the server in a different location so it no longer has the content) + + $ $PYTHON $RUNTESTDIR/killdaemons.py $DAEMON_PIDS + $ rm $DAEMON_PIDS + $ mkdir $TESTTMP/lfs-server2 + $ cd $TESTTMP/lfs-server2 +#if no-windows + $ lfs-test-server &> lfs-server.log & + $ echo $! >> $DAEMON_PIDS +#else + $ $PYTHON $TESTTMP/spawn.py >> $DAEMON_PIDS +#endif + + $ cd $TESTTMP + $ hg clone test test2 updating to branch default abort: LFS server error. Remote object for "a" not found:(.*)! (re) [255] diff --git a/hgext/lfs/blobstore.py b/hgext/lfs/blobstore.py --- a/hgext/lfs/blobstore.py +++ b/hgext/lfs/blobstore.py @@ -227,20 +227,26 @@ % rawjson) return response - def _checkforservererror(self, pointers, responses): + def _checkforservererror(self, pointers, responses, action): """Scans errors from objects Returns LfsRemoteError if any objects has an error""" for response in responses: - error = response.get('error') - if error: + # The server should return 404 when objects cannot be found. Some + # server implementation (ex. lfs-test-server) does not set "error" + # but just removes "download" from "actions". Treat that case + # as the same as 404 error. + notfound = (response.get('error', {}).get('code') == 404 + or (action == 'download' + and action not in response.get('actions', []))) + if notfound: ptrmap = {p.oid(): p for p in pointers} p = ptrmap.get(response['oid'], None) - if error['code'] == 404 and p: - filename = getattr(p, 'filename', 'unknown') - raise LfsRemoteError( - _(('LFS server error. Remote object ' - 'for "%s" not found: %r')) % (filename, response)) + filename = getattr(p, 'filename', 'unknown') + raise LfsRemoteError( + _(('LFS server error. Remote object ' + 'for "%s" not found: %r')) % (filename, response)) + if 'error' in response: raise LfsRemoteError(_('LFS server error: %r') % response) def _extractobjects(self, response, pointers, action): @@ -252,21 +258,11 @@ """ # Scan errors from objects - fail early objects = response.get('objects', []) - self._checkforservererror(pointers, objects) + self._checkforservererror(pointers, objects, action) # Filter objects with given action. Practically, this skips uploading # objects which exist in the server. filteredobjects = [o for o in objects if action in o.get('actions', [])] - # But for downloading, we want all objects. Therefore missing objects - # should be considered an error. - if action == 'download': - if len(filteredobjects) < len(objects): - missing = [o.get('oid', '?') - for o in objects - if action not in o.get('actions', [])] - raise LfsRemoteError( - _('LFS server claims required objects do not exist:\n%s') - % '\n'.join(missing)) return filteredobjects To: quark, #hg-reviewers Cc: mercurial-devel _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel