git cl patch: handle missing/wrong Gerrit issue better.

git_cl.py

 #!/usr/bin/env python
 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 # Copyright (C) 2008 Evan Martin <martine@danga.com>
 
 """A git-command for integrating reviews on Rietveld and Gerrit."""
 
 from __future__ import print_function
@@ skipping 959 matching lines @@
     parsed_url = urlparse.urlparse(url)
   except ValueError:
     return fail_result
   for cls in _CODEREVIEW_IMPLEMENTATIONS.itervalues():
     tmp = cls.ParseIssueURL(parsed_url)
     if tmp is not None:
       return tmp
   return fail_result
 
 
+class GerritIssueNotExists(Exception):

[Sergiy Byelozyorov, 2016/10/10 11:41:11]

IMHO it should be either GerritIssueExistsNot or GerritIssueDoesNotExist. Up to
you.

[tandrii(chromium), 2016/10/10 15:10:14]

I appreciate your review, but I think NotExists is easier to read and 
http://cs/search/?q=NotExists(Exception) vs
http://cs/search/?q=ExistsNot(Exception)

+  def __init__(self, issue, url):
+    self.issue = issue
+    self.url = url
+    super(GerritIssueNotExists, self).__init__()
+
+  def __str__(self):
+    return 'issue %s at %s does not exist or you have no access to it' % (
+        self.issue, self.url)
+
+
 class Changelist(object):
   """Changelist works with one changelist in local branch.
 
   Supports two codereview backends: Rietveld or Gerrit, selected at object
   creation.
 
   Notes:
     * Not safe for concurrent multi-{thread,process} use.
     * Caches values from current branch. Therefore, re-use after branch change
       with great care.
@@ skipping 1261 matching lines @@
 
   def GetStatus(self):
     """Apply a rough heuristic to give a simple summary of an issue's review
     or CQ status, assuming adherence to a common workflow.
 
     Returns None if no issue for this branch, or one of the following keywords:
       * 'error'    - error from review tool (including deleted issues)
       * 'unsent'   - no reviewers added
       * 'waiting'  - waiting for review
       * 'reply'    - waiting for owner to reply to review
-      * 'not lgtm' - Code-Review -2 from at least one approved reviewer
-      * 'lgtm'     - Code-Review +2 from at least one approved reviewer
+      * 'not lgtm' - Code-Review disaproval from at least one valid reviewer
+      * 'lgtm'     - Code-Review approval from at least one valid reviewer
       * 'commit'   - in the commit queue
       * 'closed'   - abandoned
     """
     if not self.GetIssue():
       return None
 
     try:
       data = self._GetChangeDetail(['DETAILED_LABELS', 'CURRENT_REVISION'])
-    except httplib.HTTPException:
+    except (httplib.HTTPException, GerritIssueNotExists):
       return 'error'
 
     if data['status'] in ('ABANDONED', 'MERGED'):
       return 'closed'
 
     cq_label = data['labels'].get('Commit-Queue', {})
     if cq_label:
       # Vote value is a stringified integer, which we expect from 0 to 2.
       vote_value = cq_label.get('value', '0')
       vote_text = cq_label.get('values', {}).get(vote_value, '')
@@ skipping 54 matching lines @@
     """
     raise NotImplementedError()
 
   def SubmitIssue(self, wait_for_merge=True):
     gerrit_util.SubmitChange(self._GetGerritHost(), self.GetIssue(),
                              wait_for_merge=wait_for_merge)
 
   def _GetChangeDetail(self, options=None, issue=None):
     options = options or []
     issue = issue or self.GetIssue()
-    assert issue, 'issue required to query Gerrit'
-    return gerrit_util.GetChangeDetail(self._GetGerritHost(), str(issue),
+    assert issue, 'issue is required to query Gerrit'
+    data = gerrit_util.GetChangeDetail(self._GetGerritHost(), str(issue),
                                        options)
+    if not data:
+      raise GerritIssueNotExists(issue, self.GetCodereviewServer())
+    return data
 
   def CMDLand(self, force, bypass_hooks, verbose):
     if git_common.is_dirty_git_tree('land'):
       return 1
     detail = self._GetChangeDetail(['CURRENT_REVISION', 'LABELS'])
     if u'Commit-Queue' in detail.get('labels', {}):
       if not force:
         ask_for_data('\nIt seems this repository has a Commit Queue, '
                      'which can test and land changes for you. '
                      'Are you sure you wish to bypass it?\n'
@@ skipping 34 matching lines @@
     assert not nocommit
     assert not directory
     assert parsed_issue_arg.valid
 
     self._changelist.issue = parsed_issue_arg.issue
 
     if parsed_issue_arg.hostname:
       self._gerrit_host = parsed_issue_arg.hostname
       self._gerrit_server = 'https://%s' % self._gerrit_host
 
-    detail = self._GetChangeDetail(['ALL_REVISIONS'])
+    try:
+      detail = self._GetChangeDetail(['ALL_REVISIONS'])
+    except GerritIssueNotExists as e:
+      DieWithError(str(e))
 
     if not parsed_issue_arg.patchset:
       # Use current revision by default.
       revision_info = detail['revisions'][detail['current_revision']]
       patchset = int(revision_info['_number'])
     else:
       patchset = parsed_issue_arg.patchset
       for revision_info in detail['revisions'].itervalues():
         if int(revision_info['_number']) == parsed_issue_arg.patchset:
           break
@@ skipping 2903 matching lines @@
 if __name__ == '__main__':
   # These affect sys.stdout so do it outside of main() to simplify mocks in
   # unit testing.
   fix_encoding.fix_encoding()
   setup_color.init()
   try:
     sys.exit(main(sys.argv[1:]))
   except KeyboardInterrupt:
     sys.stderr.write('interrupted\n')
     sys.exit(1)