my_activity.py: Use gerrit new REST API.

my_activity.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.
 
 """Get stats about your activity.
 
 Example:
   - my_activity.py  for stats for the current week (last week on mondays).
   - my_activity.py -Q  for stats for last quarter.
@@ skipping 98 matching lines @@
   {
     'url': 'breakpad.appspot.com',
     'supports_owner_modified_query': False,
     'requires_auth': False,
     'email_domain': 'chromium.org',
   },
 ]
 
 gerrit_instances = [
   {
-    'url': 'gerrit.chromium.org',
-    'port': 29418,
+    'url': 'chromium-review.googlesource.com',
     'shorturl': 'crosreview.com',
   },
+  # TODO: chrome-internal-review requires login credentials. Enable once login
+  # support is added to this client. See crbug.com/281695.
+  #{
+  #  'url': 'chrome-internal-review.googlesource.com',
+  #  'shorturl': 'crosreview.com/i',
+  #},
   {
-    'url': 'gerrit-int.chromium.org',
+    'host': 'gerrit.chromium.org',
+    'port': 29418,
+  },
+  {
+    'host': 'gerrit-int.chromium.org',
     'port': 29419,
-    'shorturl': 'crosreview.com/i',
   },
 ]
 
 google_code_projects = [
   {
     'name': 'chromium',
     'shorturl': 'crbug.com',
   },
   {
     'name': 'chromium-os',
@@ skipping 95 matching lines @@
 
 def get_yes_or_no(msg):
   while True:
     response = raw_input(msg + ' yes/no [no] ')
     if response == 'y' or response == 'yes':
       return True
     elif not response or response == 'n' or response == 'no':
       return False
 
 
+def datetime_from_gerrit(date_string):
+  return datetime.strptime(date_string, '%Y-%m-%d %H:%M:%S.%f000')
+
+
 def datetime_from_rietveld(date_string):
   return datetime.strptime(date_string, '%Y-%m-%d %H:%M:%S.%f')
 
 
 def datetime_from_google_code(date_string):
   return datetime.strptime(date_string, '%Y-%m-%dT%H:%M:%S.%fZ')
 
 
 class MyActivity(object):
   def __init__(self, options):
@@ skipping 117 matching lines @@
   def process_rietveld_replies(replies):
     ret = []
     for reply in replies:
       r = {}
       r['author'] = reply['sender']
       r['created'] = datetime_from_rietveld(reply['date'])
       r['content'] = ''
       ret.append(r)
     return ret
 
+  @staticmethod
+  def gerrit_changes_over_ssh(instance, filters):
+    # See https://review.openstack.org/Documentation/cmd-query.html
+    # Gerrit doesn't allow filtering by created time, only modified time.
+    gquery_cmd = ['ssh', '-p', str(instance['port']), instance['host'],
+                  'gerrit', 'query',
+                  '--format', 'JSON',
+                  '--comments',
+                  '--'] + filters
+    [stdout, _] = subprocess.Popen(gquery_cmd, stdout=subprocess.PIPE,

[M-A Ruel, 2013/09/11 13:47:26]

(stdout, _)

[deymo, 2013/09/11 21:14:35]

Done.

+                                   stderr=subprocess.PIPE).communicate()

[M-A Ruel, 2013/09/11 13:47:26]

Failure will be hard to handle since you discard both the exit code and stderr.

[deymo, 2013/09/11 21:14:35]

This part of the code is using the old gerrit interface and was like that on the
codebase. I think the idea is to ignore errors on my_activity.py (for example,
if you don't use gerrit and don't have permission or a user to access it). I'd
keep this as it is.

+    issues = str(stdout).split('\n')[:-2]

[M-A Ruel, 2013/09/11 13:47:26]

splitlines()

stdout is already a str(), not need to convert explicitly.

[deymo, 2013/09/11 21:14:35]

Done.

+    return map(json.loads, issues)
+
+  @staticmethod
+  def gerrit_changes_over_rest(instance, filters):
+    # See https://gerrit-review.googlesource.com/Documentation/rest-api.html
+    # Gerrit doesn't allow filtering by created time, only modified time.
+    args = urllib.urlencode([
+        ('q', ' '.join(filters)),
+        ('o', 'MESSAGES'),
+        ('o', 'LABELS')])
+    rest_url = 'https://%s/changes/?%s' % (instance['url'], args)
+
+    req = urllib2.Request(rest_url, headers={'Accept': 'text/plain'})
+    try:
+      response = urllib2.urlopen(req)
+      stdout = response.read()
+    except urllib2.HTTPError, e:
+      print 'ERROR: Looking up %r: %s' % (rest_url, e)
+      return []
+
+    # Check that the returned JSON starts with the right marker.

[M-A Ruel, 2013/09/11 13:47:26]

s/starts/ends/

[deymo, 2013/09/11 21:14:35]

hmmm... I think that "starts" is the right option here.

+    if stdout[:5] != ")]}'\n":
+      print 'ERROR: Marker not found on REST API response: %r' % stdout[:5]
+      return []
+    return json.loads(stdout[5:])

[M-A Ruel, 2013/09/11 13:47:26]

What are you skipping exactly?

[deymo, 2013/09/11 21:14:35]

I'm skipping the string ")]}'\n" from the beginning of the HTTP response and
checking that the skipped part is what I'm expecting to be (in the if statement
right before this return).

The REST API responds with this 5 chars and then a totally valid JSON string. I
don't know why, but that's what the examples on the documentation show (and what
the server is actually sending).

+
   def gerrit_search(self, instance, owner=None, reviewer=None):
     max_age = datetime.today() - self.modified_after
     max_age = max_age.days * 24 * 3600 + max_age.seconds
+    user_filter = 'owner:%s' % owner if owner else 'reviewer:%s' % reviewer
+    filters = ['-age:%ss' % max_age, user_filter]
 
-    # See https://review.openstack.org/Documentation/cmd-query.html
-    # Gerrit doesn't allow filtering by created time, only modified time.
-    user_filter = 'owner:%s' % owner if owner else 'reviewer:%s' % reviewer
-    gquery_cmd = ['ssh', '-p', str(instance['port']), instance['url'],
-                  'gerrit', 'query',
-                  '--format', 'JSON',
-                  '--comments',
-                  '--',
-                  '-age:%ss' % str(max_age),
-                  user_filter]
-    [stdout, _] = subprocess.Popen(gquery_cmd, stdout=subprocess.PIPE,
-                                   stderr=subprocess.PIPE).communicate()
-    issues = str(stdout).split('\n')[:-2]
-    issues = map(json.loads, issues)
+    # Determine the gerrit interface to use: SSH or REST API:
+    if 'host' in instance:
+      issues = self.gerrit_changes_over_ssh(instance, filters)
+      issues = [self.process_gerrit_ssh_issue(instance, issue)
+          for issue in issues]
+    elif 'url' in instance:
+      issues = self.gerrit_changes_over_rest(instance, filters)
+      issues = [self.process_gerrit_rest_issue(instance, issue)
+          for issue in issues]
+    else:
+      raise Exception("Invalid gerrit_instances configuration.")

[M-A Ruel, 2013/09/11 13:47:26]

resto f the file use single quotes, stay consistent.

[deymo, 2013/09/11 21:14:35]

Done.

 
     # TODO(cjhopman): should we filter abandoned changes?
-    issues = [self.process_gerrit_issue(instance, issue) for issue in issues]
     issues = filter(self.filter_issue, issues)
     issues = sorted(issues, key=lambda i: i['modified'], reverse=True)
 
     return issues
 
-  def process_gerrit_issue(self, instance, issue):
+  def process_gerrit_ssh_issue(self, instance, issue):
     ret = {}
     ret['review_url'] = issue['url']
     if 'shorturl' in instance:
       ret['review_url'] = 'http://%s/%s' % (instance['shorturl'],
                                             issue['number'])
     ret['header'] = issue['subject']
     ret['owner'] = issue['owner']['email']
     ret['author'] = ret['owner']
     ret['created'] = datetime.fromtimestamp(issue['createdOn'])
     ret['modified'] = datetime.fromtimestamp(issue['lastUpdated'])
     if 'comments' in issue:
-      ret['replies'] = self.process_gerrit_issue_replies(issue['comments'])
+      ret['replies'] = self.process_gerrit_ssh_issue_replies(issue['comments'])
     else:
       ret['replies'] = []
     ret['reviewers'] = set()
     for reply in ret['replies']:
       if reply['author'] != ret['author']:
         ret['reviewers'].add(reply['author'])
     return ret
 
   @staticmethod
-  def process_gerrit_issue_replies(replies):
+  def process_gerrit_ssh_issue_replies(replies):
     ret = []
     replies = filter(lambda r: 'email' in r['reviewer'], replies)
     for reply in replies:
       r = {}
       r['author'] = reply['reviewer']['email']
       r['created'] = datetime.fromtimestamp(reply['timestamp'])
       r['content'] = ''
       ret.append(r)
     return ret
 
+  def process_gerrit_rest_issue(self, instance, issue):
+    ret = {}
+    ret['review_url'] = 'https://%s/%s' % (instance['url'], issue['_number'])
+    if 'shorturl' in instance:
+      # TODO: Move this short link to https once crosreview.com supports it.

[M-A Ruel, 2013/09/11 13:47:26]

TODO(deymo)

[deymo, 2013/09/11 21:14:35]

Done.

+      ret['review_url'] = 'http://%s/%s' % (instance['shorturl'],
+                                            issue['_number'])
+    ret['header'] = issue['subject']
+    ret['owner'] = issue['owner']['email']
+    ret['author'] = ret['owner']
+    ret['created'] = datetime_from_gerrit(issue['created'])
+    ret['modified'] = datetime_from_gerrit(issue['updated'])
+    if 'messages' in issue:
+      ret['replies'] = self.process_gerrit_rest_issue_replies(issue['messages'])
+    else:
+      ret['replies'] = []
+    ret['reviewers'] = set()

[M-A Ruel, 2013/09/11 13:47:26]

(optional) Could be done as a one liner: 
ret['reviewers'] = set(
    r['author'] for r in ret['replies'] if r['author'] != ret['author'])

or:
ret['reviewers'] = set(r['author'] for r in ret['replies'])
ret['reviewers'].discard(ret['author'])

[deymo, 2013/09/11 21:14:35]

Changed in both functions.

On 2013/09/11 13:47:26, M-A Ruel wrote:
Done.

+    for reply in ret['replies']:
+      if reply['author'] != ret['author']:
+        ret['reviewers'].add(reply['author'])
+    return ret
+
+  @staticmethod
+  def process_gerrit_rest_issue_replies(replies):
+    ret = []
+    replies = filter(lambda r: 'email' in r['author'], replies)
+    for reply in replies:
+      r = {}

[M-A Ruel, 2013/09/11 13:47:26]

ret.append(
    {
      'author': reply['author']['email'],
      ...
    })

or better and shorter:
return [
  {
    'author': reply['author']['email'],
    ...
  }
  for reply in replies
]

[deymo, 2013/09/11 21:14:35]

This code is essentially the same as the previously existing
process_gerrit_issue_replies. I replaced both, the new and the old function.

On 2013/09/11 13:47:26, M-A Ruel wrote:

+      r['author'] = reply['author']['email']
+      r['created'] = datetime_from_gerrit(reply['date'])
+      r['content'] = reply['message']
+      ret.append(r)
+    return ret
+
   def google_code_issue_search(self, instance):
     time_format = '%Y-%m-%dT%T'
     # See http://code.google.com/p/support/wiki/IssueTrackerAPI
     # q=<owner>@chromium.org does a full text search for <owner>@chromium.org.
     # This will accept the issue if owner is the owner or in the cc list. Might
     # have some false positives, though.
 
     # Don't filter normally on modified_before because it can filter out things
     # that were modified in the time period and then modified again after it.
     gcode_url = ('https://code.google.com/feeds/issues/p/%s/issues/full' %
@@ skipping 577 matching lines @@
   print '\n\n\n'
 
   my_activity.print_changes()
   my_activity.print_reviews()
   my_activity.print_issues()
   return 0
 
 
 if __name__ == '__main__':
   sys.exit(main())