Page MenuHomePhabricator

On Phabricator workboard, show status of associated Gerrit patches
Open, LowPublic

Description

As a follow up to all-hands. @Jdrewniak wrote a Chrome extension to enhance Phabricator workboards and display status of Gerrit patches for each task.

Extension page: https://chrome.google.com/webstore/detail/pherrit/polcefipbgcdfkpbmmbdjgkgfgjoebij
Source code: https://github.com/j4n-co/pherrit

From discussions @hashar, it seems everyone is pretty much on board with integrating the feature. I can imagine it would be fairly straightforward to add it as-is. Bonus point for adding a user option which lets one enable/disable it.

Pherrit view:

Event Timeline

hashar created this task.Feb 4 2019, 10:26 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 4 2019, 10:26 AM
Mainframe98 added a subscriber: Mainframe98.
greg triaged this task as Low priority.Feb 11 2019, 6:12 AM
greg removed a project: Gerrit.
greg moved this task from To Triage to Needs code (in Phab or bot) on the Phabricator board.
greg added a subscriber: greg.

Does the extension only work on a hard-coded set of workboards? I installed it and tried to see how it would behave with some tasks in workboards I care about but I only got it to work at https://phabricator.wikimedia.org/project/view/3792/

@greg there are a few problems that I know of that could prevent it from working:

  1. The patch has multiple tasks associated with it (i.e. Bug: T1234, T12345 in the commit message)- currently the link will only show up for the first task (this can be fixed).
  2. The board has hundreds of tasks - Gerrit limits the query length to 50 terms, so work board with more than 50 tasks won't work :/ I could potentially split the queries into multiple chunks though.

Most kanban boards will work though, works for me on releng kanban, another thing to note is that the Gerrit response is huuuge. A single item from the api response looks like this:

"{
  "id": "mediawiki%2Fcore~master~I4dfee10326485e98b028585c7da2e6b30787bb91",
  "project": "mediawiki/core",
  "branch": "master",
  "hashtags": [],
  "change_id": "I4dfee10326485e98b028585c7da2e6b30787bb91",
  "subject": "Persist sessions pre-send instead of post-send",
  "status": "MERGED",
  "created": "2019-02-06 22:37:46.000000000",
  "updated": "2019-02-09 02:25:51.000000000",
  "submitted": "2019-02-09 02:21:22.000000000",
  "submitter": {
    "_account_id": 75
  },
  "insertions": 7,
  "deletions": 2,
  "unresolved_comment_count": 1,
  "has_review_started": true,
  "_number": 488598,
  "owner": {
    "_account_id": 16
  },
  "labels": {
    "Verified": {
      "all": [
        {
          "value": 0,
          "_account_id": 34
        },
        {
          "value": 2,
          "date": "2019-02-09 02:21:22.000000000",
          "permitted_voting_range": {
            "min": 2,
            "max": 2
          },
          "_account_id": 75
        }
      ],
      "values": {
        "-1": "Fails",
        " 0": "No score",
        "+1": "Checked",
        "+2": "Verified"
      },
      "default_value": 0
    },
    "Code-Review": {
      "all": [
        {
          "value": 2,
          "date": "2019-02-09 01:56:44.000000000",
          "permitted_voting_range": {
            "min": 2,
            "max": 2
          },
          "_account_id": 34
        },
        {
          "value": 0,
          "_account_id": 75
        }
      ],
      "values": {
        "-2": "Do not submit",
        "-1": "There's a problem with this change, please improve",
        " 0": "No score",
        "+1": "Looks good to me, but someone else must approve",
        "+2": "Looks good to me, approved"
      },
      "default_value": 0
    }
  },
  "removable_reviewers": [],
  "reviewers": {
    "REVIEWER": [
      {
        "_account_id": 34
      },
      {
        "_account_id": 75
      }
    ],
    "CC": [
      {
        "_account_id": 221
      }
    ]
  },
  "pending_reviewers": {},
  "tracking_ids": [
    {
      "system": "Bugzilla",
      "id": "214471"
    },
    {
      "system": "Phab",
      "id": "T214471"
    }
  ]
}"

The raw payload for releng kanban for example is over 300kb, (28kb gzipped) so it can still take a good couple of seconds for the response to load. Not sure if I can use a lighter API endpoint or trim that response 🧐 currently the query looks like this and uses something called DETAILED_LABELS...

https://gerrit.wikimedia.org/r/changes/?pp=0&o=TRACKING_IDS&o=DETAILED_LABELS&q=bug:T215861+OR+T198810+OR+T198342+OR+T215085+OR+T214688+OR+T214687+OR+T214686+OR+T214685+OR+T205258+OR+T214480+OR+T206670+OR+T214471+OR+T214389+OR+T214382+OR+T207046+OR+T214158+OR+T210285+OR+T189549+OR+T213268+OR+T204871+OR+T211904+OR+T210412+OR+T211881+OR+T211710+OR+T208529+OR+T207335+OR+T199263+OR+T199262+OR+T205923+OR+T209914+OR+T209913+OR+T188742+OR+T199113+OR+T207695+OR+T189567+OR+T191771+OR+T207669+OR+T206622+OR+T196093+OR+T182759+OR+T205002+OR+T204762+OR+T203701+OR+T203698+OR+T179963+OR+T203092+OR+T203091+OR+T94322+OR+T201875+OR+T199207+OR+T199388+OR+T179190+OR+T199116+OR+T183999+OR+T196517+OR+T196516+OR+T196436+OR+T196096+OR+T175184+OR+T193533+OR+T193222+OR+T119371+OR+T192042+OR+T182160+OR+T187733+OR+T186496+OR+T175183+OR+T183513+OR+T177867+OR+T178663+OR+T181833+OR+T214556+OR+T215079+OR+T214478+OR+T212454+OR+T211625+OR+T137890+OR+T35515+OR+T149924+OR+T212247+OR+T198968+OR+T206621+OR+T205143+OR+T190891+OR+T195244+OR+T205132+OR+T199130+OR+T208426+OR+T203846+OR+T209361+OR+T214441+OR+T205313+OR+T203084+OR+T193824+OR+T182832+OR+T182085+OR+T209946+OR+T196347+OR+T186494
greg added a comment.Feb 12 2019, 10:56 PM

Most kanban boards will work though, works for me on releng kanban, another thing to note is that the Gerrit response is huuuge.

Nice, I must have not waited for the releng-kanban one or didn't load it (I'm pretty sure I tried another workboard that probably had way more than 50 tasks :)).

Some hints for @Jdrewniak


q=bug:T215861+OR+T198810+OR+T198342

Probably each task should be prefixed with bug: eg: bug:T123 OR bug:T456 ..., that might makes it a bit faster. The Bug: T1234 headers metadata are extracted/indexed and the search should be faster with bug:T123 compared to T123 which would lookup in commit message and whatever other fields are in the search index :)

At least right now, the time goes down from ~ 2.5 seconds to 1.5 second.


The internal limit for /changes/ is 50, but if there are any more changes fetcheable, the last item would have a _more_changes: true set, doc https://gerrit.wikimedia.org/r/Documentation/rest-api-changes.html#list-changes :

If the number of changes matching the query exceeds either the internal limit or a supplied n query parameter, the last change object has a _more_changes: true JSON field set.

The S or start query parameter can be supplied to skip a number of changes from the list.

Clients are allowed to specify more than one query by setting the q parameter multiple times. In this case the result is an array of arrays, one per query in the same order the queries were given in.

Last time I have deal with the API, I would get a list. Check whether the last element has _more_changes: true. If so, count the number of elements returned (should be 50) and use that as a step to fetch the rest. Doing eg: start=50, if _more_changes, start=100 and so on until there is no more _more_changes: true :)

:)

@Jdrewniak I have seen someone complaining about reaching an url with bug:T123 OR bug:T456. I assume that came from Pherrit so I guess that one got solved? Seems to be https://github.com/j4n-co/pherrit/commit/a0f3f7e52c8aa482897efa7b4c3e9a8246acf2a9

As the limit of 50 changes being fixed by checking for _more_changes: true?

@hashar thanks for the tip about the bug parameter! I didn't fix the issue with the multiple bugs on one patch you mention above, but it did make the queries faster. That issue is caused primarily because I'm mapping the patches to tasks 1:1 here, (Array.find) when it should be mapping to an array of tasks (Array.filter).

As to the 50 task limit, the _more_changes: true parameter will come in handy, for sure, but I think what I have to do first is to add multiple query parameters to the request, each with max 50 items, something like:

https://gerrit.wikimedia.org/r/changes/?pp=0&o=TRACKING_IDS&o=DETAILED_LABELS&q={{50 items}}&q={{50 items}}&q={{50 items}}

It is good to see enhancements are being made :] For the limit of 50 items to q=, I guess you can also consider to make those queries in parallel.