Re: [dpdk-ci] [RFC pw-ci] pw_mon: check for recheck requested comments

[Aaron Conole <aconole@redhat.com>]

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"
>> ---
>>