No support for patchset level comments in CI

[Vitaliy Lotorev]

Hi

We have Gerrit instance hooked with Jenkins and Zuul. Most jobs can be retrigged with adding comment in Gerrit.

We updated Gerrit 3.2.x to 3.3.0 and triggering the jobs by comment stopped working.

It seems that Gerrit 3.3 posts patchset level comments instead of change comments [1-2].

Both Jenkins gerrit-trigger-plugin and Zuul doesn't (yet) listen to patchset level comments.

Is there any easy-to-apply patch for Gerrit users could use temporarily while CI systems start supporting patchset level comments?

ps. Checked that 'gerrit review -m ...' SSH command still creates a change message and Jenkins/Zuul jobs are triggered on it.

 

[1] https://gerrit-review.googlesource.com/c/gerrit/+/266176

[2] https://gerrit-review.googlesource.com/c/homepage/+/288131

--

Regards,

Vitaliy

[Han-Wen Nienhuys]

On Sat, Dec 5, 2020 at 10:15 PM Vitaliy Lotorev <lot...@gmail.com> wrote:
>
> Hi
> We have Gerrit instance hooked with Jenkins and Zuul. Most jobs can be retrigged with adding comment in Gerrit.
> We updated Gerrit 3.2.x to 3.3.0 and triggering the jobs by comment stopped working.
> It seems that Gerrit 3.3 posts patchset level comments instead of change comments [1-2].
> Both Jenkins gerrit-trigger-plugin and Zuul doesn't (yet) listen to patchset level comments.
>
> Is there any easy-to-apply patch for Gerrit users could use temporarily while CI systems start supporting patchset level comments?

Can you see if it works if you revert 0430b58f18ec3edead5c95297333b4cbf64d5914 ?

As you can see from the commit, there is support for targeted
switching of these features in the frontend code, but it needs a bit
of backend code to make it work. Patrick he'd look into this today,
and we could backport it to 3.3.

As an alternative, I think that messages that are ingested over email
(ie. reply to a email notification of the change) still uses classical
change messages, so you can use that as a stopgap in the meantime.

> ps. Checked that 'gerrit review -m ...' SSH command still creates a change message and Jenkins/Zuul jobs are triggered on it.
>
> [1] https://gerrit-review.googlesource.com/c/gerrit/+/266176
> [2] https://gerrit-review.googlesource.com/c/homepage/+/288131
>
> --
> Regards,
> Vitaliy
>

> --
> --
> To unsubscribe, email repo-discuss...@googlegroups.com
> More info at http://groups.google.com/group/repo-discuss?hl=en
>
> ---
> You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/repo-discuss/CALi-FuhuWBjdALUCesgRZm3N7GadzRQbnXmVKgDvYgr1DygrTA%40mail.gmail.com.



--
Han-Wen Nienhuys - Google Munich
I work 80%. Don't expect answers from me on Fridays.
--

Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich

Registergericht und -nummer: Hamburg, HRB 86891

Sitz der Gesellschaft: Hamburg

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado

[Luca Milanesio]

On 7 Dec 2020, at 13:30, 'Han-Wen Nienhuys' via Repo and Gerrit Discussion <repo-d...@googlegroups.com> wrote:

On Sat, Dec 5, 2020 at 10:15 PM Vitaliy Lotorev <lot...@gmail.com> wrote:

Hi
We have Gerrit instance hooked with Jenkins and Zuul. Most jobs can be retrigged with adding comment in Gerrit.
We updated Gerrit 3.2.x to 3.3.0 and triggering the jobs by comment stopped working.
It seems that Gerrit 3.3 posts patchset level comments instead of change comments [1-2].
Both Jenkins gerrit-trigger-plugin and Zuul doesn't (yet) listen to patchset level comments.

Is there any easy-to-apply patch for Gerrit users could use temporarily while CI systems start supporting patchset level comments?

Can you see if it works if you revert 0430b58f18ec3edead5c95297333b4cbf64d5914 ?

As you can see from the commit, there is support for targeted
switching of these features in the frontend code, but it needs a bit
of backend code to make it work. Patrick he'd look into this today,
and we could backport it to 3.3.

As an alternative, I think that messages that are ingested over email
(ie. reply to a email notification of the change) still uses classical
change messages, so you can use that as a stopgap in the meantime.

It would be also good to report this to the Gerrit Trigger Plugins and Zuul, so that they can start listening to those events as well in the future.

HTH

Luca.

To view this discussion on the web visit https://groups.google.com/d/msgid/repo-discuss/CAFQ2z_PNGSRK5Km819CVnpikRUFsuiz8_KkMBLJAyO2964SQJg%40mail.gmail.com.

[James E. Blair]

Luca Milanesio <luca.mi...@gmail.com> writes:

>> Can you see if it works if you revert
>> 0430b58f18ec3edead5c95297333b4cbf64d5914 ?
>>
>> As you can see from the commit, there is support for targeted
>> switching of these features in the frontend code, but it needs a bit
>> of backend code to make it work. Patrick he'd look into this today,
>> and we could backport it to 3.3.
>>
>> As an alternative, I think that messages that are ingested over email
>> (ie. reply to a email notification of the change) still uses classical
>> change messages, so you can use that as a stopgap in the meantime.
>
> It would be also good to report this to the Gerrit Trigger Plugins and
> Zuul, so that they can start listening to those events as well in the
> future.

It looks like Gerrit 3.3.0 doesn't include the comment text in the
stream-events output, and there also isn't a way to identify the comment
which produced the event via the REST API. That's a regression that
will prevent Jenkins and Zuul from being able to respond to comment
text.

Could we include the content of the patchset level comments in the
comment-added stream event output? Either in the comment field, or in a
new field?

Alternatively, if the message ID were included, we could look it up via
a REST API call, though that would require an extra round trip.

For reference, here is an etherpad with the event payloads in various
test scenarios: https://etherpad.opendev.org/p/zuul_gerrt-3.3_comment_recheck

-Jim

[Vitaliy Lotorev]

вт, 8 дек. 2020 г. в 01:20, Luca Milanesio <luca.mi...@gmail.com>:

On 7 Dec 2020, at 13:30, 'Han-Wen Nienhuys' via Repo and Gerrit Discussion <repo-d...@googlegroups.com> wrote:

On Sat, Dec 5, 2020 at 10:15 PM Vitaliy Lotorev <lot...@gmail.com> wrote:

Hi
We have Gerrit instance hooked with Jenkins and Zuul. Most jobs can be retrigged with adding comment in Gerrit.
We updated Gerrit 3.2.x to 3.3.0 and triggering the jobs by comment stopped working.
It seems that Gerrit 3.3 posts patchset level comments instead of change comments [1-2].
Both Jenkins gerrit-trigger-plugin and Zuul doesn't (yet) listen to patchset level comments.

Is there any easy-to-apply patch for Gerrit users could use temporarily while CI systems start supporting patchset level comments?

Can you see if it works if you revert 0430b58f18ec3edead5c95297333b4cbf64d5914 ?

As you can see from the commit, there is support for targeted
switching of these features in the frontend code, but it needs a bit
of backend code to make it work. Patrick he'd look into this today,
and we could backport it to 3.3.

As an alternative, I think that messages that are ingested over email
(ie. reply to a email notification of the change) still uses classical
change messages, so you can use that as a stopgap in the meantime.

It would be also good to report this to the Gerrit Trigger Plugins and Zuul, so that they can start listening to those events as well in the future.

Created ticket for Jenkins Gerrit Trigger [1] and the Zuul team seems to have joined this discussion.

[1] https://issues.jenkins.io/browse/JENKINS-64372

--

Regards,

Vitaliy

[Luca Milanesio]

On 8 Dec 2020, at 15:55, Vitaliy Lotorev <lot...@gmail.com> wrote:

вт, 8 дек. 2020 г. в 01:20, Luca Milanesio <luca.mi...@gmail.com>:

On 7 Dec 2020, at 13:30, 'Han-Wen Nienhuys' via Repo and Gerrit Discussion <repo-d...@googlegroups.com> wrote:

On Sat, Dec 5, 2020 at 10:15 PM Vitaliy Lotorev <lot...@gmail.com> wrote:

Hi
We have Gerrit instance hooked with Jenkins and Zuul. Most jobs can be retrigged with adding comment in Gerrit.
We updated Gerrit 3.2.x to 3.3.0 and triggering the jobs by comment stopped working.
It seems that Gerrit 3.3 posts patchset level comments instead of change comments [1-2].
Both Jenkins gerrit-trigger-plugin and Zuul doesn't (yet) listen to patchset level comments.

Is there any easy-to-apply patch for Gerrit users could use temporarily while CI systems start supporting patchset level comments?

Can you see if it works if you revert 0430b58f18ec3edead5c95297333b4cbf64d5914 ?

As you can see from the commit, there is support for targeted
switching of these features in the frontend code, but it needs a bit
of backend code to make it work. Patrick he'd look into this today,
and we could backport it to 3.3.

As an alternative, I think that messages that are ingested over email
(ie. reply to a email notification of the change) still uses classical
change messages, so you can use that as a stopgap in the meantime.

It would be also good to report this to the Gerrit Trigger Plugins and Zuul, so that they can start listening to those events as well in the future.

Created ticket for Jenkins Gerrit Trigger [1] and the Zuul team seems to have joined this discussion.

[1] https://issues.jenkins.io/browse/JENKINS-64372

Cool, let me work on [1] straight away.

Luca.

[Luca Milanesio]

> On 8 Dec 2020, at 15:28, James E. Blair <cor...@inaugust.com> wrote:
>
> Luca Milanesio <luca.mi...@gmail.com> writes:
>
>>> Can you see if it works if you revert
>>> 0430b58f18ec3edead5c95297333b4cbf64d5914 ?
>>>
>>> As you can see from the commit, there is support for targeted
>>> switching of these features in the frontend code, but it needs a bit
>>> of backend code to make it work. Patrick he'd look into this today,
>>> and we could backport it to 3.3.
>>>
>>> As an alternative, I think that messages that are ingested over email
>>> (ie. reply to a email notification of the change) still uses classical
>>> change messages, so you can use that as a stopgap in the meantime.
>>
>> It would be also good to report this to the Gerrit Trigger Plugins and
>> Zuul, so that they can start listening to those events as well in the
>> future.
>
> It looks like Gerrit 3.3.0 doesn't include the comment text in the
> stream-events output, and there also isn't a way to identify the comment
> which produced the event via the REST API. That's a regression that
> will prevent Jenkins and Zuul from being able to respond to comment
> text.
>
> Could we include the content of the patchset level comments in the
> comment-added stream event output? Either in the comment field, or in a
> new field?

Yeah, that’s the only fix we can do, well spotted Jim.
Let me have a look at that.

>
> Alternatively, if the message ID were included, we could look it up via
> a REST API call, though that would require an extra round trip.

That would be overkill IMHO.

>
> For reference, here is an etherpad with the event payloads in various
> test scenarios: https://etherpad.opendev.org/p/zuul_gerrt-3.3_comment_recheck

Thanks for sharing it, will have a look right now.

P.S. I believe we are missing one test in our Gerrit suite: the comments added to a change should be included in the stream events. If it’s not tested, it is broken, you taught me that last year :-)

Luca.

[Luca Milanesio]

We are missing *a lot more* than one test: I could not find one single test on stream events … going to raise an issue right now and writing the very first test on it.

Luca.

Luca.

[Luca Milanesio]

I’ve created [1] for the functional regression and [2] for the missing stream-events coverage.
You can watch the two issues to get updates on the progress.

Going to upload an initial change that demonstrates the problem shortly.

Luca.

[1] https://bugs.chromium.org/p/gerrit/issues/detail?id=13800
[2] https://bugs.chromium.org/p/gerrit/issues/detail?id=13799

>
> Luca.
>
>>
>> Luca.