Issue 680692 link

Starred by 1 user

Issue metadata

Status:	Fixed
Owner:	vapier@chromium.org
Closed:	Jan 2017
Cc:	briannorris@chromium.org vapier@chromium.org
Components:	Infra>Client
EstimatedDays:	----
NextAction:	----
OS:	Chrome
Pri:	3
Type:	Bug
VerifyIn-58 VerifyIn-59 VerifyIn-60 VerifyIn-61

'gerrit mine' is broken for merged chromium/google accounts

Project Member Reported by briannorris@chromium.org, Jan 12 2017

Issue description

See:

$ gerrit mine
13:10:37: WARNING: Permission error; talk to the admins of the GoB instance
GET /a/changes/?q=%28%20owner%3Abriannorris%40google.com%20OR%20owner%3Abriannorris%40chromium.org%20%29%20status%3Anew&n=500&o=DETAILED_ACCOUNTS&o=ALL_REVISIONS&o=DETAILED_LABELS&o=CURRENT_COMMIT&o=CURRENT_REVISION HTTP/1.1
HTTP/1.1 400 Bad Request
Response body: 'User briannorris@google.com not found\n'
X-ErrorId: None
13:10:37: WARNING: conn.sock.getpeername(): ('2607:f8b0:400e:c04::52', 443, 0, 0)
13:10:37: ERROR: (400) Bad Request

It looks like chromite assumes that one cares about a Gerrit account for both their chromium.org and google.com accounts. From scripts/gerrit.py:

def _MyUserInfo():
  email = git.GetProjectUserEmail(constants.CHROMITE_DIR)
  [username, _, domain] = email.partition('@')
  if domain in ('google.com', 'chromium.org'):
    emails = ['%s@%s' % (username, domain)
              for domain in ('google.com', 'chromium.org')]
  else:
    emails = [email]
  reviewers = ['reviewer:%s' % x for x in emails]
  owners = ['owner:%s' % x for x in emails]
  return emails, reviewers, owners


This must have worked previously, but now GoB isn't liking querying a non-existent account. I can ping the GoB folks about this if we really want to retain this behavior, but...do we really want to assume what someone's chromium.org vs. google.com is? Seems like a bad step. Among the reasons:

(a) FOO@chromium.org isn't guaranteed to match FOO@google.com
(b) FOO@chromium.org might not even have a google.com account

So conversely: do we expect people to do a lot of work with two different Gerrit accounts like this?

Comment 1 by vapier@chromium.org, Jan 13 2017

it mattered in the past because people wouldn't link their accounts, which means it still matters today as not everyone links.  it's also used when you look at internal-vs-external GoB -- external tends to be @chromium.org while internal tends to be @google.com.

if you delete the auto-expand logic now, it most likely will break for everyone when querying against the internal host.  that could be mitigated by changing the logic to only return the username (i.e. return "vapier"), but that would cause excessive hits if the username happens to be used by someone else via a diff domain (and have actually contributed patches).

your point about not everyone having the same username for @chromium.org and @google.com still applies.

frankly, there's never been a good answer here, which is why the current code over estimates usernames.  in the past, i don't think GoB threw an error for accounts that didn't exist (or which are registered to the same user) which is why it tended to "just work".

we could switch it to use the "self" keyword which would guarantee correct behavior for linked accounts, but for people who haven't linked, they'd be out of luck and would have to run their own queries (instead of using the "mine" shortcut).

Project Member

Comment 2 by bugdroid1@chromium.org, Jan 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/2cd560204fe7f3178ed637def7d902d6953f84cb

commit 2cd560204fe7f3178ed637def7d902d6953f84cb
Author: Mike Frysinger <vapier@chromium.org>
Date: Fri Jan 13 01:56:27 2017

gerrit: use "self" keyword for mine/todo lists

GoB has changed where looking up non-existent accounts in queries throws
an error.  Since we only expand account names automatically to deal with
unlinked accounts, and we can't always assume that devs use the same name
with @chromium.org and @google.com, replace the logic with "self".

For people with linked accounts, this "just" works all the time, and no
longer yields CLs written by other devs (if they happen to have username
conflicts across domains).  For people with unlinked accounts, this will
no longer show CLs posted by their other accounts.  Since there is not a
good answer for that scenario, and there are few devs that applies to,
require them to use manual queries instead if they want everything.

BUG= chromium:680692 
TEST=`gerrit mine` and `gerrit -i todo` still work (for me)

Change-Id: Ib4f99d0fabe0cbdba2791f428a8e5d124018ae73
Reviewed-on: https://chromium-review.googlesource.com/428018
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Tested-by: Brian Norris <briannorris@chromium.org>
Reviewed-by: Brian Norris <briannorris@chromium.org>

[modify] https://crrev.com/2cd560204fe7f3178ed637def7d902d6953f84cb/scripts/gerrit.py