Gerrit tags and approvals

[Logan Hanks]

We're working on adding a toggle to hide tagged comments in PolyGerrit (similar to what exists in the GWT UI). We're also taking it a bit farther and changing Gerrit to add its own tags to all ChangeMessages except for untagged human reviews:

https://gerrit-review.googlesource.com/c/89132/

In the process, we've discovered that NoteDB treats tags on approvals differently than ReviewDB. Consider a sequence of approvals made by one user on one patchset:

1. Verified-1 with [tag1]

2. Verified+1 with [tag2]

Under certain conditions, NoteDB will report two approvals for the same user on the same patchset, because their tags differ. ReviewDB, on the other hand, has no capability to do this; from its perspective, the Verified+1 replaces the previous approval, regardless of tag.

From what I can see from the review of the change that introduced tags:

https://gerrit-review.googlesource.com/c/75803/

The intention of tags seem to be simply to support comment filtering. The NoteDB implementation of tags deliberately adds the tag as a component of the (user, label, patchset) key for approvals, but it seems like that was done as an implementation detail of ChangeNotes parsing rather than an intentional addition to the complexity of approvals.

I'm proposing we modify NoteDB to behave in the same way as ReviewDB. Does anyone object to this? Is anyone expecting to use tagged approvals in this particular manner that NoteDB supports?

[Zaro]

Yes. We, and probably many others, have our CI (jenkins, zuul, etc..)
re-run tests on the same patchset due to outside dependencies
(network, cloud, etc..) not working properly. So for example, our CI
will run a set of jobs on a patchset which will result in Verified-1
and sometimes we know that there was a network problem, maybe one of
our services is flaky or something, then we do a re-run of the same
jobs on the same patchset. We may want to know how often this happens
so we want to save the results of from all those reruns.

> --
> --
> To unsubscribe, email repo-discuss...@googlegroups.com
> More info at http://groups.google.com/group/repo-discuss?hl=en
>
> ---
> You received this message because you are subscribed to the Google Groups
> "Repo and Gerrit Discussion" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to repo-discuss...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

[Logan Hanks]

So, for a given patchset you might have the same CI account giving different votes on the same label across different tags. How is this information surfacing to you? Do you just infer this from looking at the ChangeMessage log?

> --
> --
> To unsubscribe, email repo-discuss+unsubscribe@googlegroups.com

> More info at http://groups.google.com/group/repo-discuss?hl=en
>
> ---
> You received this message because you are subscribed to the Google Groups
> "Repo and Gerrit Discussion" group.
> To unsubscribe from this group and stop receiving emails from it, send an

> email to repo-discuss+unsubscribe@googlegroups.com.

[Zaro]

Yes, that is correct. We will occasionally rerun our jobs multiple
times on the same patchset[1] (runs triggered with 'recheck' in reply
message). The only way we can get the history of votes now is from
the comment history which isn't great because it's just unstructured
text. What we *would like* is to have Gerrit store the result for
every CI run and allow us to retrieve the entire history of results
for a patchset. This is actually one of the use cases that the
verify-status plugin[1] attempts to solve. However it currently uses
a DB to store the tests results instead of using NoteDB.

[1] https://review.openstack.org/#/c/359416/
[2] https://gerrit.googlesource.com/plugins/verify-status/

>> > To unsubscribe, email repo-discuss...@googlegroups.com

>> > More info at http://groups.google.com/group/repo-discuss?hl=en
>> >
>> > ---
>> > You received this message because you are subscribed to the Google
>> > Groups
>> > "Repo and Gerrit Discussion" group.
>> > To unsubscribe from this group and stop receiving emails from it, send
>> > an

>> > email to repo-discuss...@googlegroups.com.

[Logan Hanks]

Unless you're on NoteDB, I think you have to rely on comment history.

With the way NoteDB implements tags on approvals, you can query /changes/{change-id}/reviewers/{account-id}/votes/ and it would list them all (even if they are on the same label), provided the code hasn't changed since the vote, etc. If you someday upgrade to NoteDB, do you think you'd use that approach?

If we want to stick with the current NoteDB behavior going forward, we need to decide what deleting a vote means. If two votes by the same user on the same label exist with different tags, the only way to delete one would be to post a review with the right tag that zeros out the approval (e.g. Verified=0). We would need to add tag to DeleteVoteInput, or just change it so it deletes all approvals for the given account and label regardless of tag.

>> > To unsubscribe, email repo-discuss+unsubscribe@googlegroups.com

>> > More info at http://groups.google.com/group/repo-discuss?hl=en
>> >
>> > ---
>> > You received this message because you are subscribed to the Google
>> > Groups
>> > "Repo and Gerrit Discussion" group.
>> > To unsubscribe from this group and stop receiving emails from it, send
>> > an

>> > email to repo-discuss+unsubscribe@googlegroups.com.