From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 4345B43C21; Thu, 7 Mar 2024 18:06:19 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id EB07A42EF8; Thu, 7 Mar 2024 18:06:18 +0100 (CET) Received: from mail-yb1-f178.google.com (mail-yb1-f178.google.com [209.85.219.178]) by mails.dpdk.org (Postfix) with ESMTP id 6496B42ED7 for ; Thu, 7 Mar 2024 18:06:17 +0100 (CET) Received: by mail-yb1-f178.google.com with SMTP id 3f1490d57ef6-dd10ebcd702so1038125276.2 for ; Thu, 07 Mar 2024 09:06:17 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1709831176; x=1710435976; darn=dpdk.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=8C5exqypKI6mn4Gqc+VlmWAKk5T9752F3LHoZFxjEj0=; b=aVX3bBk9+OBshXz8glvfuspHD8tdzw1U2WLi9vAr139vgV6yci9keIYumb1P8jO9er av7coZblMkNXfvGsa33DFRSbKz//xkP/r16zOYALvAbUENeetfFj2Eo0Gv7HOn5+FFzd xws63D+Kw4NmCLmHqTZRC16cmLrMr2xUPPQ3Q= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709831177; x=1710435977; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=8C5exqypKI6mn4Gqc+VlmWAKk5T9752F3LHoZFxjEj0=; b=cnjrPPEJglUiV4UkrOQqedMBQKiwP0LDM4CqwuGGD0LE4bj3xnIOw+M2oBjJlAfhUH 4hzxScG9SEzfLc3Rv+ghJS1Ao08KSRCCbJGzmWWCU+ydR9frHV36ikNdNl9cDPed7N3N TyyfwYhAmfMd65edhoAtFan+CFiKDHndLqWLa0ywEDCHLXKWQlfjWJ4tbYuWfdQGGpZ+ H+IpEM0ZTc1YO4JopjtyRGd/DmYl3Mcg5olLrC4rQbT1DY2WmIwcEKXAz/oh5mpLqUgU +/TNLTwkL6+12rexElRRCw/OT96eOqgbSMC78r5VjjZnsplBz2YPAgxzlX5aHsOdOCXl TyZw== X-Forwarded-Encrypted: i=1; AJvYcCWahMb4egBcG+9SZ5jJF1tefnaTBll6QrjlwNBShyCciVQMXnv2b6iFI2TlUQe/skOjH2OTTv2DEY+9DHA= X-Gm-Message-State: AOJu0Yxx1XnwGIRvOc2dKaJmGuMrwKZ74CUXMEZ+W/4d0Bnx2OCTRyi/ oRh4/RqDOBDh2PUQZEL+c0sC7kIeyxNYr4wJUPgtGRIbcY/5Le01kbCoeGlqlSaZy76QWJAA/f0 m/t+e+zBWawnOVRgr2TCCC11zWvZH45xTaaWHVXKl6db1IdM0RR0= X-Google-Smtp-Source: AGHT+IFN6whaaz1ZdOTvCqlYW7ldNPTldy16DMrV3kZJfr5lr5uGN7ch1BM6aBivJ+WQ18NJZnHs/dAo3Be3amYoRhw= X-Received: by 2002:a25:aa8a:0:b0:dce:2e9:a637 with SMTP id t10-20020a25aa8a000000b00dce02e9a637mr18279096ybi.20.1709831176676; Thu, 07 Mar 2024 09:06:16 -0800 (PST) MIME-Version: 1.0 References: <2640cd5b-ea3d-cd74-d5c0-eb776e880b13@loongson.cn> In-Reply-To: From: Adam Hassick Date: Thu, 7 Mar 2024 12:06:06 -0500 Message-ID: Subject: Re: Email based retest request process: proposal for new pull/re-apply feature To: Aaron Conole Cc: zhoumin , Patrick Robb , ci@dpdk.org, dev@dpdk.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Mon, Mar 4, 2024 at 10:22=E2=80=AFAM Aaron Conole w= rote: > > zhoumin writes: > > > On Wed, Feb 21, 2024 at 2:24AM, Patrick Robb wrote: > > > > On Tue, Feb 20, 2024 at 1:12=E2=80=AFPM Aaron Conole wrote: > > > > Why not something like: > > > > Recheck-request: [attribute-list],[test-list]... > > > > For example, then we can do: > > > > Recheck-request: rebase=3D[identifier],.... > > > > where identifier is a branch specifier (or the word 'latest')? > > > > I hadn't thought about the option of allowing branch specifiers. Agree= that allowing a human correction process for > > the pw_maintainer_cli.py script choosing the wrong branch sounds helpf= ul. > > > > My original idea was offering 2 options (test original artifact, or re= -apply on latest). Do we want to support for > > checking out to a specific commit and re-applying there? I figured tha= t would not be worth it (too much of a niche > > case), but your comments are making me reconsider. > > > > I agree with you that allowing developers to correct the target branch = is useful. But, the developer should just provide > > the name of branch instead of commit ID, which is more reasonable. Of c= ourse, the rebasing option is more important. > > So, I consider we can allow developers to submit a request as following= format: > > > > Recheck-request: rebase=3DTrue|branch=3Dmain|contexts=3Diol-compile-amd= 64-testing, iol-broadcom-Performance,... > > > > We can use "|" as the separator, for example. `rebase` and `branch` can= be optional and we can use the default values > > if the developer doesn't provide them. The default is not rebasing for = `rebase` option. The default is the branch chosen > > by pw_maintainer_cli.py script for `branch` option. The `contexts` opti= on is required. > > Interesting approach. But I don't know about contexts=3D or something > like that. It means there are two passes through the regex. > > Also, I don't know about contexts either - if the series was requested > to rebase, every lab that can re-test probably should since the results > aren't going to be valid from the old tests. I'm not opposed to having the contexts be a key-value pair argument like the others, however that does break backwards compatibility with our existing syntax. If we don't care very much about backwards compatibility, then we could make this change. Instead of having a boolean and a string parameter for whether to rebase and the branch to rebase on, we could have a single argument specifying a branch. Then, labs rebase on the given branch and then rerun all tests if the "rebase=3D" argument is present. This would look like: Recheck-request: rebase=3Dmain, iol-sample-apps-testing, iol-unit-amd64-testing, iol-broadcom-Performance Or, using zhoumin's proposed format: Recheck-request: rebase=3Dmain|contexts=3Diol-sample-apps-testing, iol-unit-amd64-testing, iol-broadcom-Performance I don't think the context should be required if the request includes the rebase argument, because we do not want to mix valid and invalid test results as Aaron said. This would be a valid format if contexts are optional: Recheck-request: rebase=3Dmain > > Just spit-balling on syntax. > > > > That said, I agree - if a rebase has been requested, all tests need to > > be rerun. Maybe we should consider that the test labels should be add= ed > > with a run number or something? Or we could also include that the run > > is a rerun. That way for labs that don't currently support the rechec= k > > request framework, we can easily tell that they weren't re-tried. > > > > so re-report with a modified test label? That is good in that it shows= the behavior more clearly. But, it also means > > we will not overwrite any fails. So the fail will still be there, and = the patchwork patch page will grow a huge table. > > Maybe this is fine. > > > > Re-report with a modified test label may be better. That can tell peopl= e more information about the CI testings, such as > > that the retest indeed happened. > > Just back from PTO - actually, I don't think we need to adjust the > label, but rather the description. That would allow the mechanism that > overwrites the existing test to keep the "checks" page tidy, but also > making the retest information clear. WDYT? Yes, if the Community Lab posted new contexts for every retest then the checks page would get quite long. > > Also raises the point of getting more coverage for the retest framewor= k at other labs. I will email Min Zhou > > regarding how he uses the dpdk-ci project for the loongson build jobs = and see how well that can integrate with the > > get_reruns.py script. > > That would be great! >