facebook/sapling
1
1
Fork 0
mirror of https://github.com/facebook/sapling.git synced 2024-10-07 23:38:50 +03:00
Code Issues Projects Releases Wiki Activity

phabstatus: remove O(n^2) double loop

Browse Source 
Summary:
This was unneeded. We can just populate the dict directly.
Note that now we catch any errors in the expected data format gracefully
and no longer stack trace with bad inputs.

Reviewed By: DurhamG

Differential Revision: D6826455

fbshipit-source-id: adc9cc1fc4895f3c67b112d914f566601336ce3b
This commit is contained in:
Ryan McElroy 2018-02-07 01:53:52 -08:00 committed by Saurabh Singh
parent 57daa71514
commit 297c6ce041
4 changed files with 39 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
hgext/extlib/phabricator/graphql.py
View File

@ -97,7 +97,7 @@ class Client(object):
else:
params = { 'params': { 'numbers': rev_numbers } }
ret = self._client.query(timeout, self._getquery(), params)
return self._processrevisioninfo(ret, rev_numbers)
return self._processrevisioninfo(ret)
def _getquery(self):
return '''
@ -127,7 +127,7 @@ class Client(object):
}
'''
def _processrevisioninfo(self, ret, rev_numbers):
def _processrevisioninfo(self, ret):
try:
errormsg = ret['errors'][0]['message']
raise ClientError(None, errormsg)
@ -135,11 +135,11 @@ class Client(object):
pass
infos = {}
for revision in rev_numbers:
info = {}
for node in ret['data']['query'][0]['results']['nodes']:
if node['number'] != revision:
continue
try:
nodes = ret['data']['query'][0]['results']['nodes']
for node in nodes:
info = {}
infos[str(node['number'])] = info
status = node['diff_status_name']
# GraphQL uses "Closed" but Conduit used "Committed" so let's
@ -162,5 +162,8 @@ class Client(object):
key=operator.itemgetter('time'),
reverse=True)
info['hash'] = localcommits[0].get('commit', None)
infos[str(revision)] = info
except (TypeError, KeyError):
raise ClientError(None, 'Unexpected graphql response format')
return infos

6
tests/test-fb-hgext-diff-since-last-arc-diff.t
View File

@ -35,8 +35,10 @@ Now progressively test the response handling for variations of missing data
$ cat > $TESTTMP/mockduit << EOF
> [{}]
> EOF
$ HG_ARC_CONDUIT_MOCK=$TESTTMP/mockduit hg diff --since-last-arc-diff 2>&1 | grep Error
KeyError: 'data'
$ HG_ARC_CONDUIT_MOCK=$TESTTMP/mockduit hg diff --since-last-arc-diff
Error calling graphql: Unexpected graphql response format
abort: unable to determine previous changeset hash
[255]
$ cat > $TESTTMP/mockduit << EOF
> [{"data": {"query": [{"results": {"nodes": [{

20
tests/test-fb-hgext-phabstatus.t
View File

@ -28,8 +28,10 @@ And now with bad responses:
$ cat > $TESTTMP/mockduit << EOF
> [{}]
> EOF
$ HG_ARC_CONDUIT_MOCK=$TESTTMP/mockduit hg log -T '{phabstatus}\n' -r . 2>&1 | grep KeyError
KeyError: 'data'
$ HG_ARC_CONDUIT_MOCK=$TESTTMP/mockduit hg log -T '{phabstatus}\n' -r .
Error talking to phabricator. No diff information can be provided.
Error info: Unexpected graphql response format
Error
$ cat > $TESTTMP/mockduit << EOF
> [{"errors": [{"message": "failed, yo"}]}]
@ -42,17 +44,17 @@ And now with bad responses:
$ cat > $TESTTMP/mockduit << EOF
> [{"data": {"query": [{"results": {"nodes": null}}]}}]
> EOF
$ HG_ARC_CONDUIT_MOCK=$TESTTMP/mockduit hg log -T '{phabstatus}\n' -r . 2>&1 | grep Error
$ HG_ARC_CONDUIT_MOCK=$TESTTMP/mockduit hg log -T '{phabstatus}\n' -r .
Error talking to phabricator. No diff information can be provided.
Error info: 'NoneType' object is not iterable
Error info: Unexpected graphql response format
Error
$ cat > $TESTTMP/mockduit << EOF
> [{"data": {"query": [{"results": null}]}}]
> EOF
$ HG_ARC_CONDUIT_MOCK=$TESTTMP/mockduit hg log -T '{phabstatus}\n' -r . 2>&1 | grep Error
$ HG_ARC_CONDUIT_MOCK=$TESTTMP/mockduit hg log -T '{phabstatus}\n' -r .
Error talking to phabricator. No diff information can be provided.
Error info: 'NoneType' object has no attribute '__getitem__'
Error info: Unexpected graphql response format
Error
Missing status field is treated as an error
@ -61,8 +63,10 @@ Missing status field is treated as an error
> {"number": 1}
> ]}}]}}]
> EOF
$ HG_ARC_CONDUIT_MOCK=$TESTTMP/mockduit hg log -T '{phabstatus}\n' -r . 2>&1 | grep KeyError
KeyError: 'diff_status_name'
$ HG_ARC_CONDUIT_MOCK=$TESTTMP/mockduit hg log -T '{phabstatus}\n' -r .
Error talking to phabricator. No diff information can be provided.
Error info: Unexpected graphql response format
Error
And finally, the success case

18
tests/test-fb-hgext-syncstatus.t
View File

@ -28,8 +28,10 @@ And now with bad responses:
$ cat > $TESTTMP/mockduit << EOF
> [{}]
> EOF
$ HG_ARC_CONDUIT_MOCK=$TESTTMP/mockduit hg log -T '{syncstatus}\n' -r . 2>&1 | grep Error
KeyError: 'data'
$ HG_ARC_CONDUIT_MOCK=$TESTTMP/mockduit hg log -T '{syncstatus}\n' -r .
Error talking to phabricator. No diff information can be provided.
Error info: Unexpected graphql response format
Error
$ cat > $TESTTMP/mockduit << EOF
> [{"errors": [{"message": "failed, yo"}]}]
@ -54,8 +56,10 @@ Missing status field is treated as an error
> "differential_diffs": {"count": 3}
> }]}}]}}]
> EOF
$ HG_ARC_CONDUIT_MOCK=$TESTTMP/mockduit hg log -T '{syncstatus}\n' -r . 2>&1 | grep Error
KeyError: 'diff_status_name'
$ HG_ARC_CONDUIT_MOCK=$TESTTMP/mockduit hg log -T '{syncstatus}\n' -r .
Error talking to phabricator. No diff information can be provided.
Error info: Unexpected graphql response format
Error
Missing count field is treated as an error
@ -72,8 +76,10 @@ Missing count field is treated as an error
> }
> }]}}]}}]
> EOF
$ HG_ARC_CONDUIT_MOCK=$TESTTMP/mockduit hg log -T '{syncstatus}\n' -r . 2>&1 | grep Error
KeyError: 'differential_diffs'
$ HG_ARC_CONDUIT_MOCK=$TESTTMP/mockduit hg log -T '{syncstatus}\n' -r .
Error talking to phabricator. No diff information can be provided.
Error info: Unexpected graphql response format
Error
Missing hash field is treated as unsync