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.
Description
Description
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | • demon | T141448 Gerritbot no longer adding Patch-For-Review | |||
| Invalid | None | T141887 GerritBot not always commenting on changes in Phabricator tasks |
Event Timeline
Comment Actions
Sorry for noise. It's not discoverable that they are disjunctive (beside they shouldn't).
Comment Actions
Could this be fixed in gerrit 2.12.4 https://gerrit.googlesource.com/gerrit/+/8cf2a56bda3a55f40ef009fa726478fd1e67967e%5E%21/ ?
Comment Actions
@demon are there any errors that pin point to something in the its-phabricator plugin?
Comment Actions
@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.
Comment Actions
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.
Comment Actions
@QChris yes please. COuld you create the jar over the weekend and upload it to https://gerrit.wikimedia.org/r/#/admin/projects/operations/debs/gerrit please?
Comment Actions
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): https://people.wikimedia.org/~demon/its-phabricator-20160803/its-phabricator.jar
Comment Actions
And put upstream too so we won't lose this later: https://gerrit-review.googlesource.com/#/c/80880/
Comment Actions
@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.
<qchris> 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?
Comment Actions
This is now fixed with the rebuilt version. I'll get it in the package for next update so it's not lost.
Comment Actions
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.
Comment Actions
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?
Comment Actions
@Zppix that would not be possible since recheck is only supported in zuul. Gerrit dosent actually support recheck, we added that support in zuul.
Comment Actions
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).