Page MenuHomePhabricator

Gerritbot no longer adding Patch-For-Review
Closed, ResolvedPublic


It seems that T141241: @gerritbot not active anymore in Phabricator since Gerrit 2.12 upgrade was closed after the lack of @gerritbot comments entirely was fixed, but it's still not adding Patch-For-Review.

Event Timeline

Danny_B edited projects, added GerritBot; removed Gerrit.
Danny_B added a subscriber: Danny_B.

Sorry for noise. It's not discoverable that they are disjunctive (beside they shouldn't).

greg triaged this task as High priority.Aug 1 2016, 5:03 PM
greg added a subscriber: greg.

(higher than normal within gerritbot's backlog)

@demon are there any errors that pin point to something in the its-phabricator plugin?

@demon I carnt find add-project anywhere in the its-phabricator or its-base plugin.

We need add-project to add the Patch-For-Review to task.

We may need to re add support in the plugin again.

It seems the needed patch never made it upstream as we've been stuck on a downgraded its-phabricator plugin.

The needed patch (for the 2.8 plugin) is at P3635 (we have no local its-phabricator repo, hence abusing a paste).
If someone feels adventurous, rebase that patch on top its-phabricator master (I expect it does not apply cleanly) and build. That should give the needed jar.

Otherwise, I can prepare a new jar over the weekend.

@QChris yes please. COuld you create the jar over the weekend and upload it to please?

Here's a rebased patch on top of master:

I rebuilt the jar file. Please verify if they look good to you, and if so we'll get them deployed asap (built against v2.12.3 of core and stable-2.12 of its-base):

@demon could we try the new updated plugin on production per

<qchris> If you need it today, or tomorrow, I'd say that there is not so much risk in trying to test the new jar in production. Gerrit can load/unload them at runtime. So there should not be downtime. If the plugin fails/dies, its-phabricator is not working for 5 minutes or so, while you roll-back.
> But if it's pressing, that should be an acceptable risk.
<qchris__> I expect that it just goes fine.

@demon since I tested I carnt seem to get the correct token but I had that previously but no errors with plugin failing. So could we try it please?

demon claimed this task.

This is now fixed with the rebuilt version. I'll get it in the package for next update so it's not lost.

It doesn't seem fixed. See e.g. T142191 from today.

Yep seems it is broken again.

@demon do you have the log, to see what failed, unless the plugin managed to revert it self please.

Could we implement a fail safe to recheck commits that have been submitted to make sure gerritbot didnt miss any or would that slow performance?

@Zppix that would not be possible since recheck is only supported in zuul. Gerrit dosent actually support recheck, we added that support in zuul.

Seems to work now (cf. T142705).

However, it would be yet nicer, if it could do it in a single step (= when adding the message about existance of the patch).

Hm, yeah, I guess it works again…