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" ---
> -----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" > --- >
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
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" >> --- >>
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).
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
[-- 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 --]