New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 800232 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 784368



Sign in to add a comment

Return Tricium-generated UUID with robot comments

Project Member Reported by emso@chromium.org, Jan 9 2018

Issue description

Comment 1 by emso@chromium.org, Jan 10 2018

Running the dev server and adding more logging, I can see that the UUID is included in the RobotCommentInput: 

2018/01/10 12:55:40 INFO: Created robot comment, UUID: fc802ca3-571a-4592-81ab-4409985054f7, Analyzer: Hello, Run ID: 5733953138851840
2018/01/10 12:55:40 INFO: Created robot comment, UUID: deb78ac6-3978-46fb-97a4-2259eef79da2, Analyzer: Hello, Run ID: 5733953138851840
2018/01/10 12:55:40 DEBUG: [gerrit] JSON body: {"robot_comments":{"README.md":[{"robot_id":"Hello","robot_run_id":"5733953138851840","id":"fc802ca3-571a-4592-81ab-4409985054f7","path":"README.md","message":"Hello"}],"data.proto":[{"robot_id":"Hello","robot_run_id":"5733953138851840","id":"deb78ac6-3978-46fb-97a4-2259eef79da2","path":"data.proto","message":"Hello"}]}}
2018/01/10 12:55:40 INFO: Using Gerrit Set Review URL: https://chromium-review.googlesource.com/a/changes/playground%2Fgerrit-tricium~master~Ief88cf9b766cd714f15a136a4ac7a93030616868/revisions/ea0625d9473a3322519ece5bb90e45686495baaf/review

I think we believe that the behavior of Tricium here is correct.

The issue is that in comment (CommentInfo) objects served on the PolyGerrit UI contain different IDs. To reproduce:

1. Naviate to https://chromium-review.googlesource.com/c/playground/gerrit-tricium/+/702314/18/data.proto
2. On JS console, inspect the value of `$('gr-diff-comment').comment`.
3. Observe that it has a property id with the value "ac149ab4_e5828e30".
Blocking: 784368
Cc: emso@chromium.org
Owner: qyears...@chromium.org
Summary: Return Tricuim-generated UUID with robot comments (was: Return UUID with robot comments and not ID)
Note: all comment objects in PolyGerrit for both robot and non-robot comments contain id properties with values that are 16 hex digits with an underscore separator, and the id in $('gr-diff-comment').comment is not derived from the UUID passed in in the id field of RobotCommentInput.

Looking at the docs for the id field of CommentInput, it says:

  id (optional): The URL encoded UUID of the comment if an existing draft comment should be updated

So, I'm guessing that this id field is ignored, unless it is equal to an existing comment's 16-hex-digit ID, in which case an existing comment is updated. Since we want Tricium to track comments which are created by Tricium, I think it probably makes sense to have a "Tricium comment ID" originating from Tricium. Then I think there are two possibilities:

 1. Change Gerrit to use given comment ID when creating new comments from RobotCommentInput, and/or
 2. Store put the Tricium-generated UUID in a field in the properties map.

Reference: https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#comment-input

Summary: Return Tricium-generated UUID with robot comments (was: Return Tricuim-generated UUID with robot comments)
Project Member

Comment 6 by bugdroid1@chromium.org, Jan 18 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra/+/7889baf287c3ba8fb20b62f9d15af4a70bdfc4f1

commit 7889baf287c3ba8fb20b62f9d15af4a70bdfc4f1
Author: Quinten Yearsley <qyearsley@chromium.org>
Date: Thu Jan 18 17:11:46 2018

Add Tricium comment UUID to robot comment properties

Comment IDs surfaced in PolyGerrit don't reflect the IDs used
when posting robot comments from Tricium; I believe that this
is intentional and the id field is not meant to be provided
when posting new comments.

This CL would add the ID into a field in properties, in order
to surface the Tricium-generated ID to the frontend so that
it's accessible to the feedback button.

Bug:  800232 
Change-Id: Ia0df65bf77240177cddff0c415b506c2b8859c1e
Reviewed-on: https://chromium-review.googlesource.com/872070
Reviewed-by: Emma Söderberg <emso@chromium.org>
Commit-Queue: Quinten Yearsley <qyearsley@chromium.org>

[modify] https://crrev.com/7889baf287c3ba8fb20b62f9d15af4a70bdfc4f1/go/src/infra/tricium/appengine/gerrit/gerrit_test.go
[modify] https://crrev.com/7889baf287c3ba8fb20b62f9d15af4a70bdfc4f1/go/src/infra/tricium/appengine/gerrit/gerrit.go

Status: Fixed (was: Assigned)
I verified that these UUIDs are making it to the frontend now, under `properties.tricium_comment_uuid` in CommentInfo objects.

Sign in to add a comment