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.

Details

Related Gerrit Patches:

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 6 2017, 12:07 PM

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

Paladox added a subscriber: Paladox.Mar 6 2017, 3:23 PM
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!

hashar added a subscriber: hashar.Mar 8 2017, 9:46 AM

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 :-/

hashar added a comment.Mar 8 2017, 9:48 AM
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.

Hi is this still a problem?

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

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

Oh nope can be lowered to either normal or low.

hashar removed a subscriber: hashar.Apr 18 2017, 6:38 PM
Paladox closed this task as Resolved.Feb 12 2018, 3:32 PM
Paladox claimed this task.

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

Paladox removed a subscriber: gerritbot.

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

Paladox added a subscriber: demon.Feb 12 2018, 4:00 PM

@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.