DPDK CI discussions
 help / color / mirror / Atom feed
* [dpdk-ci] [RFC pw-ci] pw_mon: check for recheck requested comments
@ 2021-05-19 14:17 Aaron Conole
  2021-05-20  9:37 ` Juraj Linkeš
  2021-05-20 13:38 ` David Marchand
  0 siblings, 2 replies; 7+ messages in thread
From: Aaron Conole @ 2021-05-19 14:17 UTC (permalink / raw)
  To: Michael Santana
  Cc: ci, Juraj Linkes, Bruce Richardson, Thomas Monjalon, Lincoln Lavoie

ENOTREADY: Missing the actual recheck logic... needs some input /
           design before committing to anything.

When a developer wants to ask for a test case recheck (for example,
maybe to rerun the github-actions test suite), we scan for the specific
line:

^Recheck-request: .*$

The line would break up as:

   Recheck-request: [context]

where '[context]' is the name of the check (as it appears in the UI).
For example, if we look at a patch that has 'github-robot', we can
request a recheck of the series by sending an email reply with the line:

Recheck-request: github-robot

It is important to use the 'msgid' field to distinguish recheck
requests.  Otherwise, we will continuously reparse the same
recheck request and loop forever.  Additionally, we've discussed using a
counter to limit the recheck requests to a single 'recheck' per test
name.

As an additional change, we run after the 'superseded' and 'completed'
checks, to ensure that we don't bother parsing comments from older
series that aren't relevant any longer.

Signed-off-by: Aaron Conole <aconole@redhat.com>
---
Submitting to the ci@dpdk.org mailing list for inputs / comments, etc.

diff --git a/pw_mon b/pw_mon
index 28feb8b..26c667d 100755
--- a/pw_mon
+++ b/pw_mon
@@ -154,7 +154,35 @@ function check_superseded_series() {
     done
 }
 
+function run_recheck() {
+    local recheck_name=$(echo "$7" | sed 's,^Recheck-request: ,,')
+    echo "# recheck for $recheck_name requested...."
+}
+
+function check_series_needs_retest() {
+    local pw_instance="$1"
+
+    series_get_active_branches "$pw_instance" | while IFS=\| read -r series_id project url repo branchname; do
+        local patch_comments_url=$(curl -s "$userpw" "$url" | jq -rc '.comments')
+        if [ "Xnull" != "X$patch_comments_url" ]; then
+            local comments_json=$(curl -s "$userpw" "$patch_comments_url")
+            local seq_end=$(echo "$comments_json" | jq -rc 'length')
+            if [ "$seq_end" -a $seq_end -gt 0 ]; then
+                seq_end=$((seq_end-1))
+                for comment_id in $(seq 0 $seq_end); do
+                    local recheck_requested=$(echo "$comments_json" | jq -rc ".[$comment_id].content" | grep "^Recheck-request: ")
+                    if [ "X$recheck_requested" != "X" ]; then
+                        local msgid=$(echo "$comments_json" | jq -rc ".[$comment_id].msgid")
+                        run_recheck "$pw_instance" "$series_id" "$project" "$url" "$repo" "$branchname" "$recheck_requested" "$msgid"
+                    fi
+                done
+            fi
+        fi
+    done
+}
+
 check_undownloaded_series "$pw_instance" "$pw_project"
 check_completed_series "$pw_instance" "$pw_project"
 check_new_series "$pw_instance" "$pw_project"
 check_superseded_series "$pw_instance"
+check_series_needs_retest "$pw_instance"
---


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-ci] [RFC pw-ci] pw_mon: check for recheck requested comments
  2021-05-19 14:17 [dpdk-ci] [RFC pw-ci] pw_mon: check for recheck requested comments Aaron Conole
@ 2021-05-20  9:37 ` Juraj Linkeš
  2021-05-20 19:16   ` Aaron Conole
  2021-05-20 13:38 ` David Marchand
  1 sibling, 1 reply; 7+ messages in thread
From: Juraj Linkeš @ 2021-05-20  9:37 UTC (permalink / raw)
  To: Aaron Conole, Michael Santana
  Cc: ci, Bruce Richardson, Thomas Monjalon, Lincoln Lavoie



> -----Original Message-----
> From: Aaron Conole <aconole@redhat.com>
> Sent: Wednesday, May 19, 2021 4:18 PM
> To: Michael Santana <msantana@redhat.com>
> Cc: ci@dpdk.org; Juraj Linkeš <juraj.linkes@pantheon.tech>; Bruce Richardson
> <bruce.richardson@intel.com>; Thomas Monjalon <thomas@monjalon.net>;
> Lincoln Lavoie <lylavoie@iol.unh.edu>
> Subject: [RFC pw-ci] pw_mon: check for recheck requested comments
> 
> ENOTREADY: Missing the actual recheck logic... needs some input /
>            design before committing to anything.
> 
> When a developer wants to ask for a test case recheck (for example, maybe to
> rerun the github-actions test suite), we scan for the specific
> line:
> 
> ^Recheck-request: .*$
> 
> The line would break up as:
> 
>    Recheck-request: [context]
> 
> where '[context]' is the name of the check (as it appears in the UI).
> For example, if we look at a patch that has 'github-robot', we can request a
> recheck of the series by sending an email reply with the line:
> 
> Recheck-request: github-robot
> 

Do we want to support multiple contexts for one recheck request? Seems very useful since there could be multiple failed contexts.

> It is important to use the 'msgid' field to distinguish recheck requests.
> Otherwise, we will continuously reparse the same recheck request and loop
> forever.

How do you use this? Is this the message-id header? Or in-reply-to?

>  Additionally, we've discussed using a counter to limit the recheck
> requests to a single 'recheck' per test name.
> 

I think that if we're able to specify multiple contexts, then there's not really any reason to run multiple rechecks per patchset.

> As an additional change, we run after the 'superseded' and 'completed'
> checks, to ensure that we don't bother parsing comments from older series that
> aren't relevant any longer.
> 
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
> Submitting to the ci@dpdk.org mailing list for inputs / comments, etc.
> 
> diff --git a/pw_mon b/pw_mon
> index 28feb8b..26c667d 100755
> --- a/pw_mon
> +++ b/pw_mon
> @@ -154,7 +154,35 @@ function check_superseded_series() {
>      done
>  }
> 
> +function run_recheck() {
> +    local recheck_name=$(echo "$7" | sed 's,^Recheck-request: ,,')
> +    echo "# recheck for $recheck_name requested...."
> +}
> +
> +function check_series_needs_retest() {
> +    local pw_instance="$1"
> +
> +    series_get_active_branches "$pw_instance" | while IFS=\| read -r series_id
> project url repo branchname; do
> +        local patch_comments_url=$(curl -s "$userpw" "$url" | jq -rc '.comments')
> +        if [ "Xnull" != "X$patch_comments_url" ]; then
> +            local comments_json=$(curl -s "$userpw" "$patch_comments_url")
> +            local seq_end=$(echo "$comments_json" | jq -rc 'length')
> +            if [ "$seq_end" -a $seq_end -gt 0 ]; then
> +                seq_end=$((seq_end-1))
> +                for comment_id in $(seq 0 $seq_end); do
> +                    local recheck_requested=$(echo "$comments_json" | jq -rc
> ".[$comment_id].content" | grep "^Recheck-request: ")
> +                    if [ "X$recheck_requested" != "X" ]; then
> +                        local msgid=$(echo "$comments_json" | jq -rc
> ".[$comment_id].msgid")
> +                        run_recheck "$pw_instance" "$series_id" "$project" "$url"
> "$repo" "$branchname" "$recheck_requested" "$msgid"
> +                    fi
> +                done
> +            fi
> +        fi
> +    done
> +}
> +
>  check_undownloaded_series "$pw_instance" "$pw_project"
>  check_completed_series "$pw_instance" "$pw_project"
>  check_new_series "$pw_instance" "$pw_project"
>  check_superseded_series "$pw_instance"
> +check_series_needs_retest "$pw_instance"
> ---
> 



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-ci] [RFC pw-ci] pw_mon: check for recheck requested comments
  2021-05-19 14:17 [dpdk-ci] [RFC pw-ci] pw_mon: check for recheck requested comments Aaron Conole
  2021-05-20  9:37 ` Juraj Linkeš
@ 2021-05-20 13:38 ` David Marchand
  2021-05-20 21:05   ` Aaron Conole
  1 sibling, 1 reply; 7+ messages in thread
From: David Marchand @ 2021-05-20 13:38 UTC (permalink / raw)
  To: Aaron Conole
  Cc: Michael Santana, ci, Juraj Linkes, Bruce Richardson,
	Thomas Monjalon, Lincoln Lavoie

On Wed, May 19, 2021 at 4:18 PM Aaron Conole <aconole@redhat.com> wrote:
>
> ENOTREADY: Missing the actual recheck logic... needs some input /
>            design before committing to anything.
>
> When a developer wants to ask for a test case recheck (for example,
> maybe to rerun the github-actions test suite), we scan for the specific
> line:
>
> ^Recheck-request: .*$
>
> The line would break up as:
>
>    Recheck-request: [context]
>
> where '[context]' is the name of the check (as it appears in the UI).
> For example, if we look at a patch that has 'github-robot', we can
> request a recheck of the series by sending an email reply with the line:

It could happen that the tree was broken and we want to rerun all or a
list of tests.
Coud we accept multiple ^Recheck-request lines?
Or maybe have a magic "all" context?


>
> Recheck-request: github-robot
>
> It is important to use the 'msgid' field to distinguish recheck
> requests.  Otherwise, we will continuously reparse the same
> recheck request and loop forever.  Additionally, we've discussed using a
> counter to limit the recheck requests to a single 'recheck' per test
> name.
>
> As an additional change, we run after the 'superseded' and 'completed'
> checks, to ensure that we don't bother parsing comments from older
> series that aren't relevant any longer.

There was also an ask on filtering requesters (only maintainers and
patch authors can ask for a recheck).


-- 
David Marchand


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-ci] [RFC pw-ci] pw_mon: check for recheck requested comments
  2021-05-20  9:37 ` Juraj Linkeš
@ 2021-05-20 19:16   ` Aaron Conole
  0 siblings, 0 replies; 7+ messages in thread
From: Aaron Conole @ 2021-05-20 19:16 UTC (permalink / raw)
  To: Juraj Linkeš
  Cc: Michael Santana, ci, Bruce Richardson, Thomas Monjalon, Lincoln Lavoie

Juraj Linkeš <juraj.linkes@pantheon.tech> writes:

>> -----Original Message-----
>> From: Aaron Conole <aconole@redhat.com>
>> Sent: Wednesday, May 19, 2021 4:18 PM
>> To: Michael Santana <msantana@redhat.com>
>> Cc: ci@dpdk.org; Juraj Linkeš <juraj.linkes@pantheon.tech>; Bruce Richardson
>> <bruce.richardson@intel.com>; Thomas Monjalon <thomas@monjalon.net>;
>> Lincoln Lavoie <lylavoie@iol.unh.edu>
>> Subject: [RFC pw-ci] pw_mon: check for recheck requested comments
>> 
>> ENOTREADY: Missing the actual recheck logic... needs some input /
>>            design before committing to anything.
>> 
>> When a developer wants to ask for a test case recheck (for example, maybe to
>> rerun the github-actions test suite), we scan for the specific
>> line:
>> 
>> ^Recheck-request: .*$
>> 
>> The line would break up as:
>> 
>>    Recheck-request: [context]
>> 
>> where '[context]' is the name of the check (as it appears in the UI).
>> For example, if we look at a patch that has 'github-robot', we can request a
>> recheck of the series by sending an email reply with the line:
>> 
>> Recheck-request: github-robot
>> 
>
> Do we want to support multiple contexts for one recheck request? Seems
> very useful since there could be multiple failed contexts.

I think it makes sense.  Usually there's only one or two, but I can see
the need to re-run all of them.

Maybe we want something like:

^Recheck-request: ([a-zA-Z-],? ?)+$

So change the above to be:

 where '[context]' is the name of the check (as it appears in the UI),
 or a comma separated list of check names.
 For example, if we look at a patch that has 'github-robot', we can request a
 recheck of the series by sending an email reply with the line:

 Recheck-request: github-robot

Thoughts?

>> It is important to use the 'msgid' field to distinguish recheck requests.
>> Otherwise, we will continuously reparse the same recheck request and loop
>> forever.
>
> How do you use this? Is this the message-id header? Or in-reply-to?

This is the Message-ID header.  We want to prevent a rescan on a future
run from being considered again.  The idea would be to keep message-id
of the comment in the table related to rechecks.

>>  Additionally, we've discussed using a counter to limit the recheck
>> requests to a single 'recheck' per test name.
>> 
>
> I think that if we're able to specify multiple contexts, then there's
> not really any reason to run multiple rechecks per patchset.

Okay, that makes sense.

>> As an additional change, we run after the 'superseded' and 'completed'
>> checks, to ensure that we don't bother parsing comments from older series that
>> aren't relevant any longer.
>> 
>> Signed-off-by: Aaron Conole <aconole@redhat.com>
>> ---
>> Submitting to the ci@dpdk.org mailing list for inputs / comments, etc.
>> 
>> diff --git a/pw_mon b/pw_mon
>> index 28feb8b..26c667d 100755
>> --- a/pw_mon
>> +++ b/pw_mon
>> @@ -154,7 +154,35 @@ function check_superseded_series() {
>>      done
>>  }
>> 
>> +function run_recheck() {
>> +    local recheck_name=$(echo "$7" | sed 's,^Recheck-request: ,,')
>> +    echo "# recheck for $recheck_name requested...."
>> +}
>> +
>> +function check_series_needs_retest() {
>> +    local pw_instance="$1"
>> +
>> +    series_get_active_branches "$pw_instance" | while IFS=\| read -r series_id
>> project url repo branchname; do
>> +        local patch_comments_url=$(curl -s "$userpw" "$url" | jq -rc '.comments')
>> +        if [ "Xnull" != "X$patch_comments_url" ]; then
>> +            local comments_json=$(curl -s "$userpw" "$patch_comments_url")
>> +            local seq_end=$(echo "$comments_json" | jq -rc 'length')
>> +            if [ "$seq_end" -a $seq_end -gt 0 ]; then
>> +                seq_end=$((seq_end-1))
>> +                for comment_id in $(seq 0 $seq_end); do
>> +                    local recheck_requested=$(echo "$comments_json" | jq -rc
>> ".[$comment_id].content" | grep "^Recheck-request: ")
>> +                    if [ "X$recheck_requested" != "X" ]; then
>> +                        local msgid=$(echo "$comments_json" | jq -rc
>> ".[$comment_id].msgid")
>> +                        run_recheck "$pw_instance" "$series_id" "$project" "$url"
>> "$repo" "$branchname" "$recheck_requested" "$msgid"
>> +                    fi
>> +                done
>> +            fi
>> +        fi
>> +    done
>> +}
>> +
>>  check_undownloaded_series "$pw_instance" "$pw_project"
>>  check_completed_series "$pw_instance" "$pw_project"
>>  check_new_series "$pw_instance" "$pw_project"
>>  check_superseded_series "$pw_instance"
>> +check_series_needs_retest "$pw_instance"
>> ---
>> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-ci] [RFC pw-ci] pw_mon: check for recheck requested comments
  2021-05-20 13:38 ` David Marchand
@ 2021-05-20 21:05   ` Aaron Conole
  2021-05-21  9:38     ` David Marchand
  0 siblings, 1 reply; 7+ messages in thread
From: Aaron Conole @ 2021-05-20 21:05 UTC (permalink / raw)
  To: David Marchand
  Cc: Michael Santana, ci, Juraj Linkes, Bruce Richardson,
	Thomas Monjalon, Lincoln Lavoie

David Marchand <david.marchand@redhat.com> writes:

> On Wed, May 19, 2021 at 4:18 PM Aaron Conole <aconole@redhat.com> wrote:
>>
>> ENOTREADY: Missing the actual recheck logic... needs some input /
>>            design before committing to anything.
>>
>> When a developer wants to ask for a test case recheck (for example,
>> maybe to rerun the github-actions test suite), we scan for the specific
>> line:
>>
>> ^Recheck-request: .*$
>>
>> The line would break up as:
>>
>>    Recheck-request: [context]
>>
>> where '[context]' is the name of the check (as it appears in the UI).
>> For example, if we look at a patch that has 'github-robot', we can
>> request a recheck of the series by sending an email reply with the line:
>
> It could happen that the tree was broken and we want to rerun all or a
> list of tests.
> Coud we accept multiple ^Recheck-request lines?

I guess we can solve this with the comma separated list.

> Or maybe have a magic "all" context?

That might require more thought, but it's possible.  Do you think it
would be better than doing a comma separated list?

>>
>> Recheck-request: github-robot
>>
>> It is important to use the 'msgid' field to distinguish recheck
>> requests.  Otherwise, we will continuously reparse the same
>> recheck request and loop forever.  Additionally, we've discussed using a
>> counter to limit the recheck requests to a single 'recheck' per test
>> name.
>>
>> As an additional change, we run after the 'superseded' and 'completed'
>> checks, to ensure that we don't bother parsing comments from older
>> series that aren't relevant any longer.
>
> There was also an ask on filtering requesters (only maintainers and
> patch authors can ask for a recheck).


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-ci] [RFC pw-ci] pw_mon: check for recheck requested comments
  2021-05-20 21:05   ` Aaron Conole
@ 2021-05-21  9:38     ` David Marchand
  2021-06-17 14:38       ` Lincoln Lavoie
  0 siblings, 1 reply; 7+ messages in thread
From: David Marchand @ 2021-05-21  9:38 UTC (permalink / raw)
  To: Aaron Conole
  Cc: Michael Santana, ci, Juraj Linkes, Bruce Richardson,
	Thomas Monjalon, Lincoln Lavoie

On Thu, May 20, 2021 at 11:05 PM Aaron Conole <aconole@redhat.com> wrote:
> David Marchand <david.marchand@redhat.com> writes:
>
> > On Wed, May 19, 2021 at 4:18 PM Aaron Conole <aconole@redhat.com> wrote:
> >>
> >> ENOTREADY: Missing the actual recheck logic... needs some input /
> >>            design before committing to anything.
> >>
> >> When a developer wants to ask for a test case recheck (for example,
> >> maybe to rerun the github-actions test suite), we scan for the specific
> >> line:
> >>
> >> ^Recheck-request: .*$
> >>
> >> The line would break up as:
> >>
> >>    Recheck-request: [context]
> >>
> >> where '[context]' is the name of the check (as it appears in the UI).
> >> For example, if we look at a patch that has 'github-robot', we can
> >> request a recheck of the series by sending an email reply with the line:
> >
> > It could happen that the tree was broken and we want to rerun all or a
> > list of tests.
> > Coud we accept multiple ^Recheck-request lines?
>
> I guess we can solve this with the comma separated list.

Yes, it looks fine.

>
> > Or maybe have a magic "all" context?
>
> That might require more thought, but it's possible.  Do you think it
> would be better than doing a comma separated list?

No, an explicit list is actually better.
When a new check is added, for someone looking at the mails (maybe 2/3
weeks later), and reading just "all", he would have to know what
checks were available at the time.


-- 
David Marchand


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [dpdk-ci] [RFC pw-ci] pw_mon: check for recheck requested comments
  2021-05-21  9:38     ` David Marchand
@ 2021-06-17 14:38       ` Lincoln Lavoie
  0 siblings, 0 replies; 7+ messages in thread
From: Lincoln Lavoie @ 2021-06-17 14:38 UTC (permalink / raw)
  To: David Marchand
  Cc: Aaron Conole, Michael Santana, ci, Juraj Linkes,
	Bruce Richardson, Thomas Monjalon

[-- Attachment #1: Type: text/plain, Size: 2316 bytes --]

Hi Aaron,

For planning, we have this on our development schedule in the community lab
for the "Q3 Sprint" so that would likely happen during the second half of
the summer.  I don't think we had any specific comments on the mail /
request syntax.  I think a lot more of the devils are in the details around
storing off the information like message IDs, etc.  As well as how long
we're looking for retest requests, i.e. the "active" patches, etc.

Cheers,
Lincoln

On Fri, May 21, 2021 at 5:38 AM David Marchand <david.marchand@redhat.com>
wrote:

> On Thu, May 20, 2021 at 11:05 PM Aaron Conole <aconole@redhat.com> wrote:
> > David Marchand <david.marchand@redhat.com> writes:
> >
> > > On Wed, May 19, 2021 at 4:18 PM Aaron Conole <aconole@redhat.com>
> wrote:
> > >>
> > >> ENOTREADY: Missing the actual recheck logic... needs some input /
> > >>            design before committing to anything.
> > >>
> > >> When a developer wants to ask for a test case recheck (for example,
> > >> maybe to rerun the github-actions test suite), we scan for the
> specific
> > >> line:
> > >>
> > >> ^Recheck-request: .*$
> > >>
> > >> The line would break up as:
> > >>
> > >>    Recheck-request: [context]
> > >>
> > >> where '[context]' is the name of the check (as it appears in the UI).
> > >> For example, if we look at a patch that has 'github-robot', we can
> > >> request a recheck of the series by sending an email reply with the
> line:
> > >
> > > It could happen that the tree was broken and we want to rerun all or a
> > > list of tests.
> > > Coud we accept multiple ^Recheck-request lines?
> >
> > I guess we can solve this with the comma separated list.
>
> Yes, it looks fine.
>
> >
> > > Or maybe have a magic "all" context?
> >
> > That might require more thought, but it's possible.  Do you think it
> > would be better than doing a comma separated list?
>
> No, an explicit list is actually better.
> When a new check is added, for someone looking at the mails (maybe 2/3
> weeks later), and reading just "all", he would have to know what
> checks were available at the time.
>
>
> --
> David Marchand
>
>

-- 
*Lincoln Lavoie*
Principal Engineer, Broadband Technologies
21 Madbury Rd., Ste. 100, Durham, NH 03824
lylavoie@iol.unh.edu
https://www.iol.unh.edu
+1-603-674-2755 (m)
<https://www.iol.unh.edu>

[-- Attachment #2: Type: text/html, Size: 4189 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-06-17 14:38 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-19 14:17 [dpdk-ci] [RFC pw-ci] pw_mon: check for recheck requested comments Aaron Conole
2021-05-20  9:37 ` Juraj Linkeš
2021-05-20 19:16   ` Aaron Conole
2021-05-20 13:38 ` David Marchand
2021-05-20 21:05   ` Aaron Conole
2021-05-21  9:38     ` David Marchand
2021-06-17 14:38       ` Lincoln Lavoie

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).