Page MenuHomePhabricator

[wm-checks-api] Create a new gerrit bot for Patch Demo
Closed, ResolvedPublic

Assigned To
Authored By
Esanders
Mar 18 2023, 1:45 PM
Referenced Files
F37031393: patchdemo_more_warnings.png
May 24 2023, 7:01 PM
F37031376: patch_demo_as_warning.png
May 24 2023, 7:01 PM
F37031225: patchdemo_link_to_patches.png
May 24 2023, 3:26 PM
F37030589: patchdemo_results_expanded.png
May 23 2023, 9:17 PM
F37030586: patchdemo_results.png
May 23 2023, 9:17 PM
F37030584: patchdemo_run_tooltip.png
May 23 2023, 9:17 PM
F35814298: Zuul 2.5 report in Gerrit Checks UI
Mar 20 2023, 9:50 AM
Tokens
"Pterodactyl" token, awarded by matmarex."Barnstar" token, awarded by Esanders.

Description

Patch demo has a Phabricator bot (@PatchDemoBot) that announces new wikis to the corresponding tasks, but it has also been requested to post on the Gerrit patch too (https://github.com/MatmaRex/patchdemo/issues/285). Can a new bot be created on Gerrit called PatchDemoBot?

Event Timeline

There is no robot per see in Gerrit, the only exception is the its-phabricator which adds comments to Phabricator whenever a change refers to a task (via Bug: T#### in the commit headers).

For Patchdemo there are two ways to expose a site has been created for a given change:

Patchdemo sending a review

The system can be made to send a comment on the change preferably using REST API ( https://gerrit.wikimedia.org/r/Documentation/rest-api-changes.html#set-review ). That will requires the creation of a user account for Gerrit and mark it as a bot account (I have documented that just now at https://wikitech.wikimedia.org/wiki/Gerrit/Bot_account ). Then whenever a VM is created, the process can send a review back to users who will get an email notification. A disadvantage is the comments will be left in the git repository forever even after the VM got deleted/recycled. That is what Zuul CI (reporting as <code>jenkins-bot</code> is using.

The reported messages can be scrapped and integrated in the Gerrit Check UI (see below).

Check UI integration

An alternative is to have the patch demo service to expose a small web service that the Gerrit web ui can query. Whenever someone browse a change, the Gerrit frontend code would do a http request to patchdemo for the currently browsed change/patchset and the service would send back some json payload as to whether a wiki is available and potentially with some over details. That can then be shown below the commit message as an informational message and in the detailed view with some extra links. An example of the integration I have made for Zuul CI:

Zuul 2.5 report in Gerrit Checks UI (865×1 px, 145 KB)

This requires patchdemo to keep a state (which I guess it does in order to reclaim VM after a certain time) and add some web service to expose the state as json. A disadvantage is people do not receive an email notification since no comment is added to the change, I guess patchdemo can handle that. The Check UI can integrate a pending verification to which we can add a button which would do a request to patch demo to create the VM and provided the service exposes the step of the VM creation, the progress can exposed in the UI as well.

The Check UI integration is the preferred way since messages are not stored in the Gerrit database. The only drawback is there is no UI / email notification, but the emailing part can be handled by the bot while the UI part can be handled by the Checks UI.

I note patchdemo already has an API, for example https://patchdemo.wmflabs.org/api.php?patch=889642,73 yields:

[
  {
    "commit": "efebda9dae323e5c79ac296597a864dff28b2788",
    "parents": [
      {
        "commit": "968f282ebcbe76cfe0b67a95b1b2e808ec0e3722",
        "subject": "Personalized praise: Add notifications"
      }
    ],
    "author": {
      "name": "Martin Urbanec",
      "email": "martin.urbanec@wikimedia.cz",
      "date": "2023-03-02 14:20:33.000000000",
      "tz": 60
    },
    "committer": {
      "name": "Urbanecm",
      "email": "martin.urbanec@wikimedia.cz",
      "date": "2023-03-20 09:23:33.000000000",
      "tz": 0
    },
    "subject": "WIP: Frontend for Personalized praise module",
    "message": "WIP: Frontend for Personalized praise module\n\nCurrent implementation of the \"Send appreciation\"\nbutton requires T269310 to be resolved (see Depends-On).\n\nToDo:\n\n* Styling doesn't match the Figma specs yet\n* Add the (i) icon pop\n* Finish settings module\n\nTo consider:\n* Two sources of truth for praising message content:\n  Currently, praising message content is stored in\n  BOTH the preferences and the on-wiki page. The preferences\n  version is used to populate the Settings dialog, the on-wiki\n  one for the actual preloading. Ideally, the store will get\n  the praising message from the on-wiki page to populate settings.\n\nBug: T322443\nBug: T322446\nDepends-On: I4ee024cc4760542790319f302f42b1b2389ac897\nChange-Id: I0ba03ce7b3dffcfd2c17e58753f026e057803492\n",
    "r": 889642,
    "p": 73,
    "linkedTasks": [
      "322443",
      "322446"
    ]
  }
]

Had the API got the URL to the spawned Wiki we could show it up in the Gerrit Checks API. We could then potentially add actions to Create Delete VM.

I note patchdemo already has an API, for example https://patchdemo.wmflabs.org/api.php?patch=889642,73 yields:

[
  {
    "commit": "efebda9dae323e5c79ac296597a864dff28b2788",
    "parents": [
      {
        "commit": "968f282ebcbe76cfe0b67a95b1b2e808ec0e3722",
        "subject": "Personalized praise: Add notifications"
      }
    ],
    "author": {
      "name": "Martin Urbanec",
      "email": "martin.urbanec@wikimedia.cz",
      "date": "2023-03-02 14:20:33.000000000",
      "tz": 60
    },
    "committer": {
      "name": "Urbanecm",
      "email": "martin.urbanec@wikimedia.cz",
      "date": "2023-03-20 09:23:33.000000000",
      "tz": 0
    },
    "subject": "WIP: Frontend for Personalized praise module",
    "message": "WIP: Frontend for Personalized praise module\n\nCurrent implementation of the \"Send appreciation\"\nbutton requires T269310 to be resolved (see Depends-On).\n\nToDo:\n\n* Styling doesn't match the Figma specs yet\n* Add the (i) icon pop\n* Finish settings module\n\nTo consider:\n* Two sources of truth for praising message content:\n  Currently, praising message content is stored in\n  BOTH the preferences and the on-wiki page. The preferences\n  version is used to populate the Settings dialog, the on-wiki\n  one for the actual preloading. Ideally, the store will get\n  the praising message from the on-wiki page to populate settings.\n\nBug: T322443\nBug: T322446\nDepends-On: I4ee024cc4760542790319f302f42b1b2389ac897\nChange-Id: I0ba03ce7b3dffcfd2c17e58753f026e057803492\n",
    "r": 889642,
    "p": 73,
    "linkedTasks": [
      "322443",
      "322446"
    ]
  }
]

Had the API got the URL to the spawned Wiki we could show it up in the Gerrit Checks API. We could then potentially add actions to Create Delete VM.

The API is for the autocomplete when creating a wiki, but it could be modified to find wikis based on a patchset.

Either approach sounds good, although I wouldn't know how to implement the Check UI integration. If you know what to do there's a patch for the API up on GitHub.

I don't mind doing it, is merely about doing a fetch() processing the received data and return the CheckResults data structure described at https://gerrit.googlesource.com/gerrit/+/stable-3.5/polygerrit-ui/app/api/checks.ts (for Gerrit 3.5).

The one I have did crawling the comments is at https://gerrit.wikimedia.org/g/operations/software/gerrit/+/refs/heads/deploy/wmf/stable-3.5/plugins/wm-checks-api.js which is overcomplicated for sure. One that does a fetch then transform to the structure expected by Gerrit should not be too hard ;)

The API is now live, example: https://patchdemo.wmflabs.org/api.php?action=findwikis&change=822099

As described on https://github.com/MatmaRex/patchdemo/pull/537#issuecomment-1476446486, we are looking for an output something like:

RunSummary
Patchdemo↗️ 0483bb0894 PS3, 4 days ago
Patchdemo↗️ 2ca64449dd PS4, 2 hours ago
Patchdemo↗️ 449dd2ca64 PS4, 10 minutes ago

The patchset number can be found by iterating over the patches array in the result row and finding the entry with the same change number.

hashar renamed this task from Create a new gerrit bot for Patch Demo to [wm-checks-api] Create a new gerrit bot for Patch Demo.EditedMay 4 2023, 8:54 AM

I kind of forgot about that task since I ran the train and went on vacation in April. I think I will sprint it together with the similar request T331651: [wm-checks-api] support kindrobot.

Change 922605 had a related patch set uploaded (by Hashar; author: Hashar):

[operations/software/gerrit@deploy/wmf/stable-3.5] wm-patch-demo: initial implementation

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

I gave it a try against https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MobileFrontend/+/822099 tried to match the requests above and on GitHub. Here are the previews:


patchdemo_run_tooltip.png (463×628 px, 59 KB)
All the wikis instances found for the change are regrouped in a single Patch demo run, hovering the run gives of information (number of wikis for this change and a short description of what Patch demo is which I copy pasted from the Github repo description).


patchdemo_results.png (214×805 px, 51 KB)
Each of the wiki instances are a result, initially collapsed by Gerrit. The short description (in bold) more or less matches Ed description above T332474#8721491:

RunSummary
Patchdemo↗️ 0483bb0894 PS3, 4 days ago
Patchdemo↗️ 2ca64449dd PS4, 2 hours ago
Patchdemo↗️ 449dd2ca64 PS4, 10 minutes ago

I could not figure out how to set a relative date. Gerrit has its own Javascript (utils/date-utils.ts) similar to moment.js but I haven't dug how I can invoke that code from a pure JavaScript plugin. It does not seem exposed by Gerrit. That is for the future I guess.


patchdemo_results_expanded.png (554×804 px, 87 KB)

Clicking on one of the result expands a detailed description with a few things. A direct link to the wiki, one to the deletion interface and the list of patchset. I could not make direct link to each patchset cause Gerrit does not support a short URL having solely the change and patchset, it requires the repository name :-/

Change 922605 merged by jenkins-bot:

[operations/software/gerrit@deploy/wmf/stable-3.5] wm-patch-demo: initial implementation

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

Mentioned in SAL (#wikimedia-operations) [2023-05-24T14:13:43Z] <hashar@deploy1002> Started deploy [gerrit/gerrit@2d719f3]: wm-patch-demo: initial implementation | T332474

Mentioned in SAL (#wikimedia-operations) [2023-05-24T14:13:49Z] <hashar@deploy1002> Finished deploy [gerrit/gerrit@2d719f3]: wm-patch-demo: initial implementation | T332474 (duration: 00m 07s)

Of course patchdemo requires a CORS header:

Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at https://patchdemo.wmflabs.org/api.php?action=findwikis&change=922162. (Reason: CORS header ‘Access-Control-Allow-Origin’ missing). Status code: 200.

I have had the issue locally cause the Gerrit FE Dev Helper Chromium extension injects the header in the responses :/

Proposed with: https://github.com/MatmaRex/patchdemo/pull/563

Gerrit does not support a short URL having solely the change and patchset, it requires the repository name :-/

Turns out Gerrit does! The main page https://patchdemo.wmflabs.org/ links back to patchsets using URLs such as https://gerrit.wikimedia.org/r/c/921682/4

Change 922882 had a related patch set uploaded (by Hashar; author: Hashar):

[operations/software/gerrit@deploy/wmf/stable-3.5] wm-patch-demo: link to other patches

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

Neat! Will this only be under Checks > Info? If so, can we make it more visible/discoverable somehow?

Neat! Will this only be under Checks > Info? If so, can we make it more visible/discoverable somehow?

I think the infos can be added to the popup link for the Patch demo run:

patchdemo_run_tooltip.png (463×628 px, 59 KB)

An alternative would be to move all the spinned wikis as individual Runs

Demo wiki 1234abc
Demo wiki fee3333
Demo wiki 309449

And provide the list of patches as results, something like:

RunSummary
Demo wiki 1234abcPS4 + 1234,3 - 2023-05-24
Demo wiki 1234abcPS4+ 7777,42 - 2023-05-24
Demo wiki fee3333PS2 + 1234,3 - 2023-05-24
Demo wiki 309449PS3 + 1234,3 - 2023-05-24
Demo wiki 309449PS3 + 7777,42 - 2023-05-24

If that makes more or less sense I can rework the code and upload new images.

(I should also update the doc to make it easy for others to hack on those)

I guess I meant something immediately visible, without having to go to the "Checks" tab, so that less technical users might find it quickly, but maybe gerrit doesn't let us do that (unless you write a bot that posts a comment)?

The little chips are collapsed when there are more than 3 elements (and up to 6 for ERRORS).

If I change the category from INFO to WARNING and head to https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MobileFrontend/+/822099 (which has 4 wikis using that change and hence 4 warnings) that renders with the name:

patch_demo_as_warning.png (147×482 px, 22 KB)

And with 4 warnings:

patchdemo_more_warnings.png (138×520 px, 24 KB)

So I guess we can change the Patch demo runs from INFO to WARNING which will make them standout?

Hmm - that might be an improvement.

On a separate note, how often does it poll the API, or is it just when a new patchset is uploaded? If it's just when a new patchset is uploaded, does that mean we will always be missing a wiki created with the latest patchset?

The plugin runs 100% from the client browser, so anytime one load the change or refresh it due to a new patchset, the plugin will refresh the results. Additionally it fetches:

If you open a tab to create a wiki, wait for it to complete and then head back to the tab having the change, that triggers a fetch and the wiki will show up :]

I will send the patch for the INFO > WARNING change.

Change 923418 had a related patch set uploaded (by Hashar; author: Hashar):

[operations/software/gerrit@deploy/wmf/stable-3.5] wm-patch-demo: use WARNING to prevent chipset collapsing

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

Change 922882 merged by jenkins-bot:

[operations/software/gerrit@deploy/wmf/stable-3.5] wm-patch-demo: link to other patches

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

Change 923418 merged by jenkins-bot:

[operations/software/gerrit@deploy/wmf/stable-3.5] wm-patch-demo: use WARNING to prevent chipset collapsing

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

Mentioned in SAL (#wikimedia-operations) [2023-05-26T11:35:46Z] <hashar@deploy1002> Started deploy [gerrit/gerrit@c490ae6]: wm-patch-demo: link to other patches, use WARNING to prevent chipset collapsing | T332474

Mentioned in SAL (#wikimedia-operations) [2023-05-26T11:35:54Z] <hashar@deploy1002> Finished deploy [gerrit/gerrit@c490ae6]: wm-patch-demo: link to other patches, use WARNING to prevent chipset collapsing | T332474 (duration: 00m 08s)

Change 923589 had a related patch set uploaded (by Hashar; author: Hashar):

[operations/software/gerrit@deploy/wmf/stable-3.5] wm-patch-demo: do not return runs when there are no wikis

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

Change 923589 merged by jenkins-bot:

[operations/software/gerrit@deploy/wmf/stable-3.5] wm-patch-demo: do not return runs when there are no wikis

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

Mentioned in SAL (#wikimedia-operations) [2023-05-26T12:21:09Z] <hashar@deploy1002> Started deploy [gerrit/gerrit@0932557]: wm-patch-demo: do not return runs when there are no wikis | T332474

Mentioned in SAL (#wikimedia-operations) [2023-05-26T12:21:17Z] <hashar@deploy1002> Finished deploy [gerrit/gerrit@0932557]: wm-patch-demo: do not return runs when there are no wikis | T332474 (duration: 00m 08s)

I have merged the two patches I have made earlier and the results are now reported as WARNING which makes them stand out a bit.

I added a last minute change to hide Patch Demo entirely when there are no wikis found, which is the case for most changes in Gerrit and avoid the spammy Found 0 wikis for change XXXX.

I am marking this resolved, if there are adjustments needed feel free to reopen :]