Page MenuHomePhabricator

Fix @gerritbot patch attribution
Closed, ResolvedPublic

Description

... or at least make that the author of the patch be properly attributed (ie. not the 'shell' name if different from git config --global user.name, which should prevail). Compare T159657#3075111 (new) with T126604#2018621 (old). I certainly prefer that the patches I upload be attributed to MarcoAurelio and not to maurelio.

Event Timeline

Abandoning patches works just fine: T159657#3075242 -- I guess actions done through Gerrit GUI parses the usernames correctly.

Paladox triaged this task as High priority.Mar 7 2017, 4:07 PM

Two people have complained now. One of them said it on irc. I will try fixing it now.

Change 341559 had a related patch set uploaded (by paladox):
[operations/puppet] Gerrit: Fix bot so it uses the name of user instead of username

https://gerrit.wikimedia.org/r/341559

Change 341559 merged by Dzahn:
[operations/puppet] Gerrit: Fix bot so it uses the name of user instead of username

https://gerrit.wikimedia.org/r/341559

Thank you very much for this!

https://gerrit.wikimedia.org/r/#/c/341559/ that changed the PatchsetCreated.vm file causes some server side errors:

/var/lib/gerrit2/review_site/logs/error_log
ERROR velocity : Variable $author-username has not been set at ItsComment[line 1, column 29]
WARN  com.google.gerrit.server.extensions.events.EventUtil : Error in listener com.google.gerrit.server.events.StreamEventsApiListener for event com.google.gerrit.server.extensions.events.RevisionCreated: Variable $author-username has not been set at ItsComment[line 1, column 29]

There is no other detail though :-/

gerrit-server/src/main/java/com/google/gerrit/server/events/StreamEventsApiListener.java
@Override
public void onRevisionCreated(RevisionCreatedListener.Event ev) {
  try {
    ChangeNotes notes = getNotes(ev.getChange());
    Change change = notes.getChange();
    PatchSet patchSet = getPatchSet(notes, ev.getRevision());
    PatchSetCreatedEvent event = new PatchSetCreatedEvent(change);

    event.change = changeAttributeSupplier(change);
    event.patchSet = patchSetAttributeSupplier(change, patchSet);
    event.uploader = accountAttributeSupplier(ev.getWho());

    dispatcher.get().postEvent(change, event);
  } catch (OrmException e) {
    log.error("Failed to dispatch event", e);
  }
}

The event does not have an author-username property?

Oh but it should be using the Patchset event.

I guess we could submit a patch upstream to support that?

gerrit-server/src/main/java/com/google/gerrit/server/events/StreamEventsApiListener.java
@Override
public void onRevisionCreated(RevisionCreatedListener.Event ev) {
  try {
    ChangeNotes notes = getNotes(ev.getChange());
    Change change = notes.getChange();
    PatchSet patchSet = getPatchSet(notes, ev.getRevision());
    PatchSetCreatedEvent event = new PatchSetCreatedEvent(change);

    event.change = changeAttributeSupplier(change);
    event.patchSet = patchSetAttributeSupplier(change, patchSet);
    event.uploader = accountAttributeSupplier(ev.getWho());

    dispatcher.get().postEvent(change, event);
  } catch (OrmException e) {
    log.error("Failed to dispatch event", e);
  }
}

The event does not have an author-username property?

That's probably fixed in gerrit 2.14 as i can't reproduce on master.

@Paladox: Is this still an urgent (=high priority) problem?

Paladox lowered the priority of this task from High to Medium.Apr 18 2017, 5:06 PM

Oh nope can be lowered to either normal or low.

Paladox claimed this task.

Closing as resolved. Please reopen if the problem still happends.

Closing as resolved. Please reopen if the problem still happends.

@Paladox: Do you know that some related software changes have taken place that should solve this or not?
I do not see any patch attribution at all currently, e.g. for https://gerrit.wikimedia.org/r/#/c/409692/ in https://phabricator.wikimedia.org/T131136#3961308

@Aklapper thanks, yes i think that may be a bug, but without logs, i cannot reproduce it or even supply a fix.

@demon would you be able to have a look in the logs for ^^ please?

Possibly a soy error.