Page MenuHomePhabricator

Whitelisted users without +2 rights can no longer recheck patches which have a +2
Closed, ResolvedPublic

Description

As I’m on the CI whitelist since I67c94cbf65, I expected that my “recheck” comment on I2c929f8ef6 would have some effect. However, nothing seems to happen.

I thought that, following T105474, it would repeat the gate-and-submit build. I assume that didn’t happen because I don’t have +2 rights (not on my private account, anyways), so the gate-and-submit didn’t happen; however, I guess the part that’s responsible for scheduling the test build didn’t check that, and assumed that a recheck on a +2ed change should only trigger a gate-and-submit build, and so the test build wasn’t triggered either.

(The title of this task is based on those guesses, feel free to wildly edit it and the task description if I turn out to be wrong.)

Details

Related Gerrit Patches:

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 14 2019, 9:51 AM

The change had Code-Review +2 by Amir and Verified -1 by Jenkins.

The recheck event did match:

zuul.IndependentPipelineManager:
Event <TriggerEvent comment-added mediawiki/extensions/OAuth master 500177,4 Verified:0, Code-Review:0>
for change <Change 0x7fadd72a98d0 500177,4>
matched <EventFilter types: comment-added branches: (?!^refs/meta/config) ignore_deletes: True
^^^^^^^^
         comments: (?im)^Patch Set \d+:( -?Code\-Review(\+|-)?(1|2)?)?(\n\n\(\d+ comment(s)?\))?\n\n\s*recheck\b 
         emails: <bunch of emails>>
in pipeline <IndependentPipelineManager test>

But the test pipeline rejects it because it has a Code-Review+2:

2019-05-14 09:38:52,349 INFO zuul.Scheduler:
  Adding mediawiki/extensions/OAuth, <Change 0x7fadd72a98d0 500177,4> to <Pipeline test>
2019-05-14 09:38:52,349 DEBUG zuul.IndependentPipelineManager:
  Considering adding change <Change 0x7fadd72a98d0 500177,4>
2019-05-14 09:38:52,349 DEBUG zuul.IndependentPipelineManager:
  Change <Change 0x7fadd72a98d0 500177,4>
  does not match pipeline requirement <ChangeishFilter reject_approvals: [{'code-review': 2}]>
  ^^^^^^^^^^^^^^

That is due to T105474 / 0f0aecfb695e668786ac9847dfed63f79d8fcca6 which made test to reject Code-Review +2 changes.

The event is not processed by gate-and-submit because it only accepts comments that do have a Code-Review +2. If Amir had done a recheck that would have enqueued it.

Change 510148 had a related patch set uploaded (by Hashar; owner: Hashar):
[integration/config@master] Revert "zuul: skip test/test-prio for CR+2 changes"

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

Change 510148 merged by jenkins-bot:
[integration/config@master] Revert "zuul: skip test/test-prio for CR+2 changes"

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

hashar closed this task as Resolved.May 14 2019, 12:34 PM
hashar claimed this task.

I have reverted the change that caused the issue. Sorry for the inconvenience.

Oh no :( in my opinion the recheck convenience was totally worth this small regression!

(but perhaps the other regression was more significant, I wouldn’t know about that)

Well both were annoying regressions, albeit little ones so I guess it was better to just revert :-] Eventually I will come again to the problem and figure out a proper solution.