git cl: reduce number of Gerrit RPCs in status check.

Also fixes git cl description <url> by propagating codereview host information
in all cases.

BUG=681704
R=agable@chromium.org

Change-Id: Ia79c10b19d72b5a8797a1428ad8a79c8f4480901
Reviewed-on: https://chromium-review.googlesource.com/431036
Reviewed-by: Aaron Gable <agable@chromium.org>
changes/36/431036/10
Andrii Shyshkalov 8 years ago
parent 8fc0c1d8f0
commit 8039be7cc8

@ -1785,13 +1785,13 @@ class _ChangelistCodereviewBase(object):
class _RietveldChangelistImpl(_ChangelistCodereviewBase): class _RietveldChangelistImpl(_ChangelistCodereviewBase):
def __init__(self, changelist, auth_config=None, rietveld_server=None): def __init__(self, changelist, auth_config=None, codereview_host=None):
super(_RietveldChangelistImpl, self).__init__(changelist) super(_RietveldChangelistImpl, self).__init__(changelist)
assert settings, 'must be initialized in _ChangelistCodereviewBase' assert settings, 'must be initialized in _ChangelistCodereviewBase'
if not rietveld_server: if not codereview_host:
settings.GetDefaultServerUrl() settings.GetDefaultServerUrl()
self._rietveld_server = rietveld_server self._rietveld_server = codereview_host
self._auth_config = auth_config or auth.make_auth_config() self._auth_config = auth_config or auth.make_auth_config()
self._props = None self._props = None
self._rpc_server = None self._rpc_server = None
@ -2208,16 +2208,21 @@ class _RietveldChangelistImpl(_ChangelistCodereviewBase):
class _GerritChangelistImpl(_ChangelistCodereviewBase): class _GerritChangelistImpl(_ChangelistCodereviewBase):
def __init__(self, changelist, auth_config=None): def __init__(self, changelist, auth_config=None, codereview_host=None):
# auth_config is Rietveld thing, kept here to preserve interface only. # auth_config is Rietveld thing, kept here to preserve interface only.
super(_GerritChangelistImpl, self).__init__(changelist) super(_GerritChangelistImpl, self).__init__(changelist)
self._change_id = None self._change_id = None
# Lazily cached values. # Lazily cached values.
self._gerrit_server = None # e.g. https://chromium-review.googlesource.com
self._gerrit_host = None # e.g. chromium-review.googlesource.com self._gerrit_host = None # e.g. chromium-review.googlesource.com
self._gerrit_server = None # e.g. https://chromium-review.googlesource.com
# Map from change number (issue) to its detail cache. # Map from change number (issue) to its detail cache.
self._detail_cache = {} self._detail_cache = {}
if codereview_host is not None:
assert not codereview_host.startswith('https://'), codereview_host
self._gerrit_host = codereview_host
self._gerrit_server = 'https://%s' % codereview_host
def _GetGerritHost(self): def _GetGerritHost(self):
# Lazy load of configs. # Lazy load of configs.
self.GetCodereviewServer() self.GetCodereviewServer()
@ -2444,6 +2449,11 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
issue = issue or self.GetIssue() issue = issue or self.GetIssue()
assert issue, 'issue is required to query Gerrit' assert issue, 'issue is required to query Gerrit'
# Optimization to avoid multiple RPCs:
if (('CURRENT_REVISION' in options or 'ALL_REVISIONS' in options) and
'CURRENT_COMMIT' not in options):
options.append('CURRENT_COMMIT')
# Normalize issue and options for consistent keys in cache. # Normalize issue and options for consistent keys in cache.
issue = str(issue) issue = str(issue)
options = [o.upper() for o in options] options = [o.upper() for o in options]
@ -3926,10 +3936,10 @@ def CMDdescription(parser, args):
options, args = parser.parse_args(args) options, args = parser.parse_args(args)
_process_codereview_select_options(parser, options) _process_codereview_select_options(parser, options)
target_issue = None target_issue_arg = None
if len(args) > 0: if len(args) > 0:
target_issue = ParseIssueNumberArgument(args[0]) target_issue_arg = ParseIssueNumberArgument(args[0])
if not target_issue.valid: if not target_issue_arg.valid:
parser.print_help() parser.print_help()
return 1 return 1
@ -3939,10 +3949,9 @@ def CMDdescription(parser, args):
'auth_config': auth_config, 'auth_config': auth_config,
'codereview': options.forced_codereview, 'codereview': options.forced_codereview,
} }
if target_issue: if target_issue_arg:
kwargs['issue'] = target_issue.issue kwargs['issue'] = target_issue_arg.issue
if options.forced_codereview == 'rietveld': kwargs['codereview_host'] = target_issue_arg.hostname
kwargs['rietveld_server'] = target_issue.hostname
cl = Changelist(**kwargs) cl = Changelist(**kwargs)

@ -1677,7 +1677,7 @@ class TestGitCl(TestCase):
self.calls += [ self.calls += [
(('GetChangeDetail', git_short_host + '-review.googlesource.com', (('GetChangeDetail', git_short_host + '-review.googlesource.com',
'123456', ['ALL_REVISIONS']), '123456', ['ALL_REVISIONS', 'CURRENT_COMMIT']),
{ {
'current_revision': '7777777777', 'current_revision': '7777777777',
'revisions': { 'revisions': {
@ -2042,13 +2042,7 @@ class TestGitCl(TestCase):
out = StringIO.StringIO() out = StringIO.StringIO()
self.mock(git_cl.sys, 'stdout', out) self.mock(git_cl.sys, 'stdout', out)
self.calls = [ self.calls = [
((['git', 'symbolic-ref', 'HEAD'],), 'master'), (('GetChangeDetail', 'code.review.org',
((['git', 'config', 'branch.master.gerritserver'],), CERR1),
((['git', 'config', 'branch.master.merge'],), 'refs/heads/master'),
((['git', 'config', 'branch.master.remote'],), 'origin'),
((['git', 'config', 'remote.origin.url'],),
'https://git.review.org/repo.git'),
(('GetChangeDetail', 'git-review.review.org',
'123123', ['CURRENT_REVISION', 'CURRENT_COMMIT']), '123123', ['CURRENT_REVISION', 'CURRENT_COMMIT']),
{ {
'current_revision': 'sha1', 'current_revision': 'sha1',
@ -2382,7 +2376,7 @@ class TestGitCl(TestCase):
['DETAILED_ACCOUNTS']), ['DETAILED_ACCOUNTS']),
{'owner': {'email': 'owner@e.mail'}}), {'owner': {'email': 'owner@e.mail'}}),
(('GetChangeDetail', 'chromium-review.googlesource.com', '123456', (('GetChangeDetail', 'chromium-review.googlesource.com', '123456',
['ALL_REVISIONS']), { ['ALL_REVISIONS', 'CURRENT_COMMIT']), {
'project': 'depot_tools', 'project': 'depot_tools',
'revisions': { 'revisions': {
'deadbeaf': { 'deadbeaf': {

Loading…
Cancel
Save