[dpdk-ci] [PATCH 0/9] Automatic patchwork delegation

[dpdk-ci] [PATCH 0/9] Automatic patchwork delegation

[Ali Alnubani]

This patchset adds support for automatic patch delegation.

The script 'guess_git_tree.py' was renamed to 'maintainers.py'
and now supports finding patch and series maintainers. It can also
delegate patches to them using git-pw.

The script 'poll-pw' was rewritten to fetch new api resources
by filtering with date and project. It now supports fetching both
patches and series.

Ali Alnubani (9):
  tools: rename guess_git_tree script
  tools: match by tree url instead of tree name
  tools: update script usage
  tools: add functionality for detecting tree maintainers
  tools: add functionality for setting pw delegates
  add git-pw to requirements file
  tools: filter new patchwork IDs by date
  tools: add support for fetching new series IDs
  tools: filter new patchwork IDs by project name

 requirements.txt                            |   3 +-
 tools/{guess_git_tree.py => maintainers.py} | 113 ++++++++++++++++----
 tools/poll-pw                               |  75 ++++++++-----
 3 files changed, 148 insertions(+), 43 deletions(-)
 rename tools/{guess_git_tree.py => maintainers.py} (69%)

-- 
2.25.1

[dpdk-ci] [PATCH 1/9] tools: rename guess_git_tree script

[Ali Alnubani]

The name 'maintainers.py' will make more sense when
adding more operations to the script.

Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
---
 tools/{guess_git_tree.py => maintainers.py} | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
 rename tools/{guess_git_tree.py => maintainers.py} (98%)

diff --git a/tools/guess_git_tree.py b/tools/maintainers.py
similarity index 98%
rename from tools/guess_git_tree.py
rename to tools/maintainers.py
index c9eef39..0cf1907 100755
--- a/tools/guess_git_tree.py
+++ b/tools/maintainers.py
@@ -31,13 +31,13 @@ variables PW_{SERVER,PROJECT,TOKEN} should be set. If not, the script will try
 to load the git configurations pw.{server,project,token}.
 
 Example usage:
-    ./guess-git-tree.py --command list_trees_for_series 2054
-    ./guess-git-tree.py --command list_trees_for_patch 2054
+    ./maintainers.py --command list_trees_for_series 2054
+    ./maintainers.py --command list_trees_for_patch 2054
 
 Or if you want to use inside other scripts:
 
     import os
-    from guess_git_tree import (Maintainers, GitPW, Diff)
+    from maintainers import (Maintainers, GitPW, Diff)
     _git_pw = GitPW({
         'pw_server': os.environ.get('PW_SERVER'),
         'pw_project': os.environ.get('PW_PROJECT'),
-- 
2.25.1

Re: [dpdk-ci] [PATCH 1/9] tools: rename guess_git_tree script

[Juraj Linkeš]

> -----Original Message-----
> From: ci <ci-bounces@dpdk.org> On Behalf Of Ali Alnubani
> Sent: Monday, September 6, 2021 5:45 PM
> To: ci@dpdk.org
> Cc: thomas@monjalon.net; jerinj@marvell.com; ferruh.yigit@intel.com;
> david.marchand@redhat.com
> Subject: [dpdk-ci] [PATCH 1/9] tools: rename guess_git_tree script
> 
> The name 'maintainers.py' will make more sense when adding more operations
> to the script.
> 

A noun evokes something non-executable and in my opinion 'maintainers.py' doesn't describe what the script does in any way.

As far as I can tell, the script (with all the new changes) gets data from patchwork and the maintainers file and then processes those to produce a list of trees, maintainers or updates PW. Maybe name it pw_maintainers_cli.py?

Another thing that would make sense it to make the "--command" argument positional, since it's mandatory and the argument name isn't really necessary. But maybe that's a change for another series.

> Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
> ---
>  tools/{guess_git_tree.py => maintainers.py} | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)  rename tools/{guess_git_tree.py
> => maintainers.py} (98%)
> 
> diff --git a/tools/guess_git_tree.py b/tools/maintainers.py similarity index 98%
> rename from tools/guess_git_tree.py rename to tools/maintainers.py index
> c9eef39..0cf1907 100755
> --- a/tools/guess_git_tree.py
> +++ b/tools/maintainers.py
> @@ -31,13 +31,13 @@ variables PW_{SERVER,PROJECT,TOKEN} should be set.
> If not, the script will try  to load the git configurations pw.{server,project,token}.
> 
>  Example usage:
> -    ./guess-git-tree.py --command list_trees_for_series 2054
> -    ./guess-git-tree.py --command list_trees_for_patch 2054
> +    ./maintainers.py --command list_trees_for_series 2054
> +    ./maintainers.py --command list_trees_for_patch 2054
> 
>  Or if you want to use inside other scripts:
> 
>      import os
> -    from guess_git_tree import (Maintainers, GitPW, Diff)
> +    from maintainers import (Maintainers, GitPW, Diff)
>      _git_pw = GitPW({
>          'pw_server': os.environ.get('PW_SERVER'),
>          'pw_project': os.environ.get('PW_PROJECT'),
> --
> 2.25.1
> 

Re: [dpdk-ci] [PATCH 1/9] tools: rename guess_git_tree script

[Ali Alnubani]

> -----Original Message-----
> From: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Sent: Tuesday, September 7, 2021 2:03 PM
> To: Ali Alnubani <alialnu@nvidia.com>; ci@dpdk.org
> Cc: NBU-Contact-Thomas Monjalon <thomas@monjalon.net>;
> jerinj@marvell.com; ferruh.yigit@intel.com; david.marchand@redhat.com
> Subject: RE: [dpdk-ci] [PATCH 1/9] tools: rename guess_git_tree script
> 
> 
> 
> > -----Original Message-----
> > From: ci <ci-bounces@dpdk.org> On Behalf Of Ali Alnubani
> > Sent: Monday, September 6, 2021 5:45 PM
> > To: ci@dpdk.org
> > Cc: thomas@monjalon.net; jerinj@marvell.com; ferruh.yigit@intel.com;
> > david.marchand@redhat.com
> > Subject: [dpdk-ci] [PATCH 1/9] tools: rename guess_git_tree script
> >
> > The name 'maintainers.py' will make more sense when adding more
> > operations to the script.
> >
> 
> A noun evokes something non-executable and in my opinion 'maintainers.py'
> doesn't describe what the script does in any way.
> 
> As far as I can tell, the script (with all the new changes) gets data from
> patchwork and the maintainers file and then processes those to produce a
> list of trees, maintainers or updates PW. Maybe name it
> pw_maintainers_cli.py?
> 
> Another thing that would make sense it to make the "--command" argument
> positional, since it's mandatory and the argument name isn't really necessary.
> But maybe that's a change for another series.

Thanks for taking a look Juraj, I agree with the name you suggested, and I'll update in v2.
I'll also try to implement your second suggestion in that version.

> 
> > Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
> > ---
> >  tools/{guess_git_tree.py => maintainers.py} | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)  rename
> > tools/{guess_git_tree.py => maintainers.py} (98%)
> >
> > diff --git a/tools/guess_git_tree.py b/tools/maintainers.py similarity
> > index 98% rename from tools/guess_git_tree.py rename to
> > tools/maintainers.py index
> > c9eef39..0cf1907 100755
> > --- a/tools/guess_git_tree.py
> > +++ b/tools/maintainers.py
> > @@ -31,13 +31,13 @@ variables PW_{SERVER,PROJECT,TOKEN} should be
> set.
> > If not, the script will try  to load the git configurations
> pw.{server,project,token}.
> >
> >  Example usage:
> > -    ./guess-git-tree.py --command list_trees_for_series 2054
> > -    ./guess-git-tree.py --command list_trees_for_patch 2054
> > +    ./maintainers.py --command list_trees_for_series 2054
> > +    ./maintainers.py --command list_trees_for_patch 2054
> >
> >  Or if you want to use inside other scripts:
> >
> >      import os
> > -    from guess_git_tree import (Maintainers, GitPW, Diff)
> > +    from maintainers import (Maintainers, GitPW, Diff)
> >      _git_pw = GitPW({
> >          'pw_server': os.environ.get('PW_SERVER'),
> >          'pw_project': os.environ.get('PW_PROJECT'),
> > --
> > 2.25.1
> >
> 

[dpdk-ci] [PATCH 2/9] tools: match by tree url instead of tree name

[Ali Alnubani]

The tree url has more information. It can be used to
get the maintainer of a tree, and it includes the tree name anyway.

Also try to use named capture groups as much as possible.

Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
---
 tools/maintainers.py | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/tools/maintainers.py b/tools/maintainers.py
index 0cf1907..22933d5 100755
--- a/tools/maintainers.py
+++ b/tools/maintainers.py
@@ -46,7 +46,8 @@ Or if you want to use inside other scripts:
     maintainers = Maintainers()
     patch_id = 52199
     files = Diff.find_filenames(_git_pw.api_get('patches', patch_id)['diff'])
-    tree = maintainers.get_tree(files)
+    tree_url = maintainers.get_tree(files)
+    tree_name = tree_url.split('/')[-1]
 """
 
 
@@ -116,9 +117,9 @@ class Diff(object):
 class Maintainers(object):
 
     file_regex = r'F:\s(.*)'
-    tree_regex = r'T: git:\/\/dpdk\.org(?:\/next)*\/(.*)'
+    tree_regex = r'T: (?P<url>git:\/\/dpdk\.org(?:\/next)*\/(?P<name>.*))'
     section_regex = r'([^\n]*)\n-+.*?(?=([^\n]*\n-+)|\Z)'
-    subsection_regex = r'[^\n](?:(?!\n{{2}}).)*?^F: {}(?:(?!\n{{2}}).)*'
+    subsection_regex = r'[^\n](?:(?!\n{{2}}).)*?^{}: {}$(?:(?!\n{{2}}).)*'
 
     def __init__(self):
         with open(MAINTAINERS_FILE_PATH) as fd:
@@ -151,8 +152,8 @@ class Maintainers(object):
             if _tree:
                 tree_list.append(_tree)
         tree = self.get_common_denominator(tree_list)
-        if tree == '':
-            tree = 'dpdk'
+        if not tree:
+            tree = 'git://dpdk.org/dpdk'
         return tree
 
     def _get_tree(self, filename):
@@ -180,7 +181,7 @@ class Maintainers(object):
 
         found_match = False
         # Find the block containing filename.
-        regex = self.subsection_regex.format(re.escape(matching_pattern))
+        regex = self.subsection_regex.format('F', re.escape(matching_pattern))
         subsection_match = re.findall(
                 regex,
                 self.maintainers_txt,
@@ -191,7 +192,7 @@ class Maintainers(object):
             tree_match = re.search(
                     self.tree_regex, subsection)
             if tree_match:
-                tree = tree_match.group(1)
+                tree = tree_match.group('url')
                 self.matched[matching_pattern] = tree
                 found_match = True
 
@@ -204,7 +205,7 @@ class Maintainers(object):
                             self.tree_regex,
                             section.group(0).split('\n\n')[0])
                     if tree_match:
-                        tree = tree_match.group(1)
+                        tree = tree_match.group('url')
 
         self.matched[matching_pattern] = tree
         return tree
@@ -228,8 +229,8 @@ class Maintainers(object):
             os.path.commonprefix(_tree_list).rstrip('-').replace(
                     'dpdk-next-net-virtio', 'dpdk-next-virtio')
         # There is no 'dpdk-next' named tree.
-        if common_prefix == 'dpdk-next':
-            common_prefix = 'dpdk'
+        if common_prefix.endswith('dpdk-next') or common_prefix.endswith('/'):
+            common_prefix = 'git://dpdk.org/dpdk'
         return common_prefix
 
 
@@ -289,4 +290,4 @@ if __name__ == '__main__':
     files = []
     for patch in patch_list:
         files += Diff.find_filenames(patch['diff'])
-    print(maintainers.get_tree(files))
+    print(maintainers.get_tree(files).split('/')[-1])
-- 
2.25.1

[dpdk-ci] [PATCH 3/9] tools: update script usage

[Ali Alnubani]

The resource type (whether it's a patch or a series) is now
specified by the new argument --type.
Both commands: list_trees_for_patch & list_trees_for_series
are replaced by the command: list_trees, and the type must
be always set.

This makes adding more operations require less arguments
and easier.

Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
---
 tools/maintainers.py | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/tools/maintainers.py b/tools/maintainers.py
index 22933d5..e4ea317 100755
--- a/tools/maintainers.py
+++ b/tools/maintainers.py
@@ -31,8 +31,8 @@ variables PW_{SERVER,PROJECT,TOKEN} should be set. If not, the script will try
 to load the git configurations pw.{server,project,token}.
 
 Example usage:
-    ./maintainers.py --command list_trees_for_series 2054
-    ./maintainers.py --command list_trees_for_patch 2054
+    ./maintainers.py --command list_trees --type series 2054
+    ./maintainers.py --command list_trees --type patch 2054
 
 Or if you want to use inside other scripts:
 
@@ -243,9 +243,14 @@ if __name__ == '__main__':
     options_parser.add_argument(
             '--command',
             choices=(
-                'list_trees_for_patch',
-                'list_trees_for_series'),
+                'list_trees'),
             required=True, help='Command to perform')
+    options_parser.add_argument(
+            '--type',
+            choices=(
+                'patch',
+                'series'),
+            required=True, help='Resource type.')
 
     git_pw_conf_parser.add_argument(
             '--pw_server', type=str,
@@ -268,6 +273,7 @@ if __name__ == '__main__':
     args = parser.parse_args()
 
     command = args.command
+    resource_type = args.type
     _id = args.id
 
     # Pass the needed configurations to git-pw.
@@ -279,9 +285,9 @@ if __name__ == '__main__':
     maintainers = Maintainers()
 
     patch_list = []
-    if command == 'list_trees_for_patch':
+    if resource_type == 'patch':
         patch_list.append(_git_pw.api_get('patches', _id))
-    elif command == 'list_trees_for_series':
+    else:
         series = _git_pw.api_get('series', _id)
         patch_list = [
                 _git_pw.api_get('patches', patch['id'])
@@ -290,4 +296,8 @@ if __name__ == '__main__':
     files = []
     for patch in patch_list:
         files += Diff.find_filenames(patch['diff'])
-    print(maintainers.get_tree(files).split('/')[-1])
+
+    tree = maintainers.get_tree(files)
+
+    if command == 'list_trees':
+        print(tree.split('/')[-1])
-- 
2.25.1

[dpdk-ci] [PATCH 4/9] tools: add functionality for detecting tree maintainers

[Ali Alnubani]

Detecting a maintainer works by searching the
'General Project Administration' section for subsections
containing the provided tree, and then returning the maintainers
specified in that subsection.

Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
---
 tools/maintainers.py | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/tools/maintainers.py b/tools/maintainers.py
index e4ea317..c9b0502 100755
--- a/tools/maintainers.py
+++ b/tools/maintainers.py
@@ -33,6 +33,7 @@ to load the git configurations pw.{server,project,token}.
 Example usage:
     ./maintainers.py --command list_trees --type series 2054
     ./maintainers.py --command list_trees --type patch 2054
+    ./maintainers.py --command list_maintainers --type patch 2054
 
 Or if you want to use inside other scripts:
 
@@ -48,6 +49,7 @@ Or if you want to use inside other scripts:
     files = Diff.find_filenames(_git_pw.api_get('patches', patch_id)['diff'])
     tree_url = maintainers.get_tree(files)
     tree_name = tree_url.split('/')[-1]
+    maintainers = maintainers.get_maintainers(tree_url)
 """
 
 
@@ -118,6 +120,7 @@ class Maintainers(object):
 
     file_regex = r'F:\s(.*)'
     tree_regex = r'T: (?P<url>git:\/\/dpdk\.org(?:\/next)*\/(?P<name>.*))'
+    maintainer_regex = r'M:\s(.*)'
     section_regex = r'([^\n]*)\n-+.*?(?=([^\n]*\n-+)|\Z)'
     subsection_regex = r'[^\n](?:(?!\n{{2}}).)*?^{}: {}$(?:(?!\n{{2}}).)*'
 
@@ -141,6 +144,26 @@ class Maintainers(object):
         # Save already matched patterns.
         self.matched = {}
 
+    def get_maintainers(self, tree):
+        """
+        Return a list of a tree's maintainers."""
+        maintainers = []
+        for section in self.sections:
+            if section.group(1) == 'General Project Administration':
+                # Find the block containing the tree.
+                regex = self.subsection_regex.format('T', re.escape(tree))
+                subsection_match = re.findall(
+                        regex,
+                        section.group(0),
+                        re.DOTALL | re.MULTILINE)
+                if len(subsection_match):
+                    subsection = subsection_match[-1]
+                    # Look for maintainers
+                    maintainers = re.findall(
+                            self.maintainer_regex, subsection)
+                    return maintainers
+                break
+
     def get_tree(self, files):
         """
         Return a git tree that matches a list of files."""
@@ -243,7 +266,8 @@ if __name__ == '__main__':
     options_parser.add_argument(
             '--command',
             choices=(
-                'list_trees'),
+                'list_trees',
+                'list_maintainers'),
             required=True, help='Command to perform')
     options_parser.add_argument(
             '--type',
@@ -301,3 +325,5 @@ if __name__ == '__main__':
 
     if command == 'list_trees':
         print(tree.split('/')[-1])
+    elif command == 'list_maintainers':
+        print(*maintainers.get_maintainers(tree), sep='\n')
-- 
2.25.1

[dpdk-ci] [PATCH 5/9] tools: add functionality for setting pw delegates

[Ali Alnubani]

A new command was added to set patch delegates in Patchwork
based on the emails found in DPDK's MAINTAINERS file.

Example usage:
  $ export MAINTAINERS_FILE_PATH=/path/to/dpdk/MAINTAINERS
  $ ./maintainers.py --command set_pw_delegate --type series SERIES_ID

Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
---
 tools/maintainers.py | 46 +++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 43 insertions(+), 3 deletions(-)

diff --git a/tools/maintainers.py b/tools/maintainers.py
index c9b0502..a1ba5fc 100755
--- a/tools/maintainers.py
+++ b/tools/maintainers.py
@@ -14,6 +14,7 @@ from requests.exceptions import HTTPError
 from git_pw import config
 from git_pw import api
 from git_pw import utils
+from git_pw import patch as git_pw_patch
 
 """
 Description:
@@ -84,6 +85,29 @@ class GitPW(object):
             else:
                 raise
 
+    def set_delegate(self, patch_list, delegate):
+        """Set the delegate for a patch.
+        Only tries to set a delegate for patches that don't have one set already.
+
+        Reference:
+        https://github.com/getpatchwork/git-pw/blob/76b79097dc0a57c89b45dd53d9cacb7ff7b31bb2/git_pw/patch.py#L167
+        """
+        users = api.index('users', [('q', delegate)])
+        if len(users) != 1:
+            # Zero or multiple users found
+            print('Cannot choose a Patchwork user to delegate to from '
+                    'user list ({}). Skipping..'.format(users))
+            return
+        for patch in patch_list:
+            if patch['delegate']:
+                print('Patch {} is already delegated to {}. '
+                        'Skipping..'.format(patch['id'], patch['delegate']['email']))
+                continue
+            print("Delegating patch {} to {}.".format(
+                patch['id'], users[0]['email']))
+            _ = api.update(
+                    'patches', patch['id'], [('delegate', users[0]['id'])])
+
 
 class Diff(object):
 
@@ -267,7 +291,8 @@ if __name__ == '__main__':
             '--command',
             choices=(
                 'list_trees',
-                'list_maintainers'),
+                'list_maintainers',
+                'set_pw_delegate'),
             required=True, help='Command to perform')
     options_parser.add_argument(
             '--type',
@@ -325,5 +350,20 @@ if __name__ == '__main__':
 
     if command == 'list_trees':
         print(tree.split('/')[-1])
-    elif command == 'list_maintainers':
-        print(*maintainers.get_maintainers(tree), sep='\n')
+    if command in ['list_maintainers', 'set_pw_delegate']:
+        maintainer_list = maintainers.get_maintainers(tree)
+        if command == 'list_maintainers':
+            print(*maintainer_list, sep='\n')
+        elif command == 'set_pw_delegate':
+            if len(maintainer_list) > 0:
+                # Get the email of the first maintainer in the list.
+                try:
+                    delegate = re.match(
+                            r".*\<(?P<email>.*)\>",
+                            maintainer_list[0]).group('email')
+                except AttributeError:
+                    print("Unexpected format: '{}'".format(maintainer_list[0]))
+                    sys.exit(1)
+                _git_pw.set_delegate(patch_list, delegate)
+            else:
+                print('No maintainers found. Not setting a delegate.')
-- 
2.25.1

[dpdk-ci] [PATCH 6/9] add git-pw to requirements file

[Ali Alnubani]

The module has always been required by tools/maintainers.py (previously
named tools/guess_git_tree.py).

Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
---
 requirements.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/requirements.txt b/requirements.txt
index f20067d..f2a6844 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1 +1,2 @@
-whatthepatch==1.0.2
\ No newline at end of file
+git-pw==2.1.0
+whatthepatch==1.0.2
-- 
2.25.1

[dpdk-ci] [PATCH 7/9] tools: filter new patchwork IDs by date

[Ali Alnubani]

API resource IDs are guaranteed to be unique, but they aren't guaranteed
to have no gaps, for example, the following series IDs are
nonexistent: 16157, 17181, 18235.
Filtering by the date since the last check is necessary to later
add support for fetching new series IDs in addition to patch IDs.

Instead of requiring a file that contains the next patch ID,
a file containing the timestamp of the last time the API was fetched
is now used.
Each time the API is fetched for new patches, the timestamp
in the file gets updated, and the script sleeps an amount of time specified
by PAUSE_SECONDS before attempting to fetch new resources again.

The pause amount between each poll attempt is still 100 seconds.

Setting the env variable 'TZ' might be necessary if your timezone
doesn't match the server's timezone.

The package jq (Command-line JSON processor) is now required by the
script.

Example usage:
$ export TZ="Europe/Paris"
$ export MAINTAINERS_FILE_PATH=/path/to/dpdk/MAINTAINERS
$ ./tools/poll-pw /path/to/last.txt \
    '/path/to/maintainers.py --command set_pw_delegate --type patch $1'

Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
---
 tools/poll-pw | 56 ++++++++++++++++++++++++++++++++-------------------
 1 file changed, 35 insertions(+), 21 deletions(-)

diff --git a/tools/poll-pw b/tools/poll-pw
index bdf860a..e104dab 100755
--- a/tools/poll-pw
+++ b/tools/poll-pw
@@ -1,58 +1,72 @@
-#! /bin/sh -e
+#! /bin/sh
 
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright 2017 6WIND S.A.
 # Copyright 2018 Mellanox Technologies, Ltd
 
 URL=http://patches.dpdk.org/api
+PAUSE_SECONDS=100
 
 print_usage () {
 	cat <<- END_OF_HELP
-	usage: $(basename $0) <counter> <command>
+	usage: $(basename $0) [OPTIONS] </path/to/last.txt> <command>
 
 	Poll patchwork and call command for each patch.
-	The first patchwork id to be checked is read from the counter file.
-	The command should use '$1' to be evaluated as patchwork id.
-	When a patch is found and the command is successful,
-	then the counter is incremented.
+	The first date to filter with is read from the specified file.
+	The command should use '$1' to be evaluated as the patch id.
+	The date in the specifed file is updated after each pull.
 	END_OF_HELP
 }
 
+which jq >/dev/null 2>&1
+if [ ! $? -eq 0 ] ; then
+	echo "The command 'jq' doesn't exist, please install it."
+	exit 1
+fi
+
 while getopts h arg ; do
 	case $arg in
 		h ) print_usage ; exit 0 ;;
 		? ) print_usage >&2 ; exit 1 ;;
 	esac
 done
+
 if [ $# -lt 2 ] ; then
-	printf 'missing argument\n\n' >&2
+	printf 'missing argument(s)\n\n' >&2
 	print_usage >&2
 	exit 1
 fi
 shift $(($OPTIND - 1))
-counter=$1
+since_file=$1
 shift
 cmd=$*
 
+if [ ! -f "$since_file" ] ; then
+	echo "The file '$since_file' doesn't exist."
+	exit 1
+fi
+
+date -d "$(cat $since_file | tr '\n' ' ')" >/dev/null 2>&1
+if [ ! $? -eq 0 ] ; then
+	echo "The file '$since_file' doesn't contain a valid date format."
+	exit 1
+fi
+
+URL="${URL}/patches/?"
+
 callcmd () # <patchwork id>
 {
 	eval $cmd
 }
 
-checkid () # <patchwork id>
-{
-	curl -sfIo /dev/null $URL/patches/$1/ ||
-	curl -sfIo /dev/null $URL/covers/$1/
-}
-
-pwid=$(cat $counter)
+set -e
 while true ; do
-	# process all recent patches
-	while checkid $pwid ; do
-		callcmd $pwid || break
-		pwid=$(($pwid + 1))
-		echo $pwid >$counter
+	date_now=$(date '+%FT%T')
+	since=$(date -d $(cat $since_file | tr '\n' ' ') '+%FT%T')
+	for id in $(curl -s "${URL}since=${since}" | jq '.[].id') ; do
+		callcmd $id || break
 	done
+	echo -n $date_now >$since_file
 	# pause before next check
-	sleep 100
+	sleep $PAUSE_SECONDS
 done
-- 
2.25.1

Re: [dpdk-ci] [PATCH 7/9] tools: filter new patchwork IDs by date

[Ali Alnubani]

> -----Original Message-----
> From: ci <ci-bounces@dpdk.org> On Behalf Of Ali Alnubani
> Sent: Monday, September 6, 2021 6:46 PM
> To: ci@dpdk.org
> Cc: NBU-Contact-Thomas Monjalon <thomas@monjalon.net>;
> jerinj@marvell.com; ferruh.yigit@intel.com; david.marchand@redhat.com
> Subject: [dpdk-ci] [PATCH 7/9] tools: filter new patchwork IDs by date
> 
> API resource IDs are guaranteed to be unique, but they aren't guaranteed to
> have no gaps, for example, the following series IDs are
> nonexistent: 16157, 17181, 18235.
> Filtering by the date since the last check is necessary to later add support for
> fetching new series IDs in addition to patch IDs.
> 
> Instead of requiring a file that contains the next patch ID, a file containing the
> timestamp of the last time the API was fetched is now used.
> Each time the API is fetched for new patches, the timestamp in the file gets
> updated, and the script sleeps an amount of time specified by
> PAUSE_SECONDS before attempting to fetch new resources again.
> 
> The pause amount between each poll attempt is still 100 seconds.
> 
> Setting the env variable 'TZ' might be necessary if your timezone doesn't
> match the server's timezone.
> 
> The package jq (Command-line JSON processor) is now required by the
> script.
> 
> Example usage:
> $ export TZ="Europe/Paris"
> $ export MAINTAINERS_FILE_PATH=/path/to/dpdk/MAINTAINERS
> $ ./tools/poll-pw /path/to/last.txt \
>     '/path/to/maintainers.py --command set_pw_delegate --type patch $1'
> 
> Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
> ---
>  tools/poll-pw | 56 ++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 35 insertions(+), 21 deletions(-)
> 
> diff --git a/tools/poll-pw b/tools/poll-pw index bdf860a..e104dab 100755
> --- a/tools/poll-pw
> +++ b/tools/poll-pw
> @@ -1,58 +1,72 @@
> -#! /bin/sh -e
> +#! /bin/sh
> 
>  # SPDX-License-Identifier: BSD-3-Clause  # Copyright 2017 6WIND S.A.
>  # Copyright 2018 Mellanox Technologies, Ltd
> 
>  URL=http://patches.dpdk.org/api
> +PAUSE_SECONDS=100
> 
>  print_usage () {
>  	cat <<- END_OF_HELP
> -	usage: $(basename $0) <counter> <command>
> +	usage: $(basename $0) [OPTIONS] </path/to/last.txt> <command>
> 
>  	Poll patchwork and call command for each patch.
> -	The first patchwork id to be checked is read from the counter file.
> -	The command should use '$1' to be evaluated as patchwork id.
> -	When a patch is found and the command is successful,
> -	then the counter is incremented.
> +	The first date to filter with is read from the specified file.
> +	The command should use '$1' to be evaluated as the patch id.
> +	The date in the specifed file is updated after each pull.
>  	END_OF_HELP
>  }
> 
> +which jq >/dev/null 2>&1
> +if [ ! $? -eq 0 ] ; then
> +	echo "The command 'jq' doesn't exist, please install it."
> +	exit 1
> +fi
> +
>  while getopts h arg ; do
>  	case $arg in
>  		h ) print_usage ; exit 0 ;;
>  		? ) print_usage >&2 ; exit 1 ;;
>  	esac
>  done
> +
>  if [ $# -lt 2 ] ; then
> -	printf 'missing argument\n\n' >&2
> +	printf 'missing argument(s)\n\n' >&2
>  	print_usage >&2
>  	exit 1
>  fi
>  shift $(($OPTIND - 1))
> -counter=$1
> +since_file=$1
>  shift
>  cmd=$*
> 
> +if [ ! -f "$since_file" ] ; then
> +	echo "The file '$since_file' doesn't exist."
> +	exit 1
> +fi
> +
> +date -d "$(cat $since_file | tr '\n' ' ')" >/dev/null 2>&1 if [ ! $?
> +-eq 0 ] ; then
> +	echo "The file '$since_file' doesn't contain a valid date format."
> +	exit 1
> +fi
> +
> +URL="${URL}/patches/?"
> +
>  callcmd () # <patchwork id>
>  {
>  	eval $cmd
>  }
> 
> -checkid () # <patchwork id>
> -{
> -	curl -sfIo /dev/null $URL/patches/$1/ ||
> -	curl -sfIo /dev/null $URL/covers/$1/
> -}
> -
> -pwid=$(cat $counter)
> +set -e
>  while true ; do
> -	# process all recent patches
> -	while checkid $pwid ; do
> -		callcmd $pwid || break
> -		pwid=$(($pwid + 1))
> -		echo $pwid >$counter
> +	date_now=$(date '+%FT%T')
> +	since=$(date -d $(cat $since_file | tr '\n' ' ') '+%FT%T')
> +	for id in $(curl -s "${URL}since=${since}" | jq '.[].id') ; do
> +		callcmd $id || break
>  	done
> +	echo -n $date_now >$since_file
>  	# pause before next check
> -	sleep 100
> +	sleep $PAUSE_SECONDS
>  done

Thomas pointed out that the script can fetch the same ID twice in some cases, and that we can miss some IDs due to time synchronization. I'll look into that.

> --
> 2.25.1

[dpdk-ci] [PATCH 8/9] tools: add support for fetching new series IDs

[Ali Alnubani]

The script can now fetch new series IDs the same way
it fetches new patch IDs.

Example usage:
$ export TZ="Europe/Paris"
$ export MAINTAINERS_FILE_PATH=/path/to/dpdk/MAINTAINERS
$ ./tools/poll-pw series /path/to/last.txt \
    '/path/to/maintainers.py --command set_pw_delegate --type series $1'

Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
---
 tools/poll-pw | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/tools/poll-pw b/tools/poll-pw
index e104dab..6017146 100755
--- a/tools/poll-pw
+++ b/tools/poll-pw
@@ -9,11 +9,11 @@ PAUSE_SECONDS=100
 
 print_usage () {
 	cat <<- END_OF_HELP
-	usage: $(basename $0) [OPTIONS] </path/to/last.txt> <command>
+	usage: $(basename $0) [OPTIONS] <series|patches> </path/to/last.txt> <command>
 
-	Poll patchwork and call command for each patch.
+	Poll patchwork and call a command for each new patch/series id.
 	The first date to filter with is read from the specified file.
-	The command should use '$1' to be evaluated as the patch id.
+	The command should use '$1' to be evaluated as the patch/series id.
 	The date in the specifed file is updated after each pull.
 	END_OF_HELP
 }
@@ -31,16 +31,22 @@ while getopts h arg ; do
 	esac
 done
 
-if [ $# -lt 2 ] ; then
+if [ $# -lt 3 ] ; then
 	printf 'missing argument(s)\n\n' >&2
 	print_usage >&2
 	exit 1
 fi
 shift $(($OPTIND - 1))
-since_file=$1
-shift
+resource_type=$1
+since_file=$2
+shift 2
 cmd=$*
 
+if ! curl -sfIo /dev/null $URL/$resource_type ; then
+	echo "API endpoint '$URL/$resource_type' doesn't exist or inaccessible."
+	exit 1
+fi
+
 if [ ! -f "$since_file" ] ; then
 	echo "The file '$since_file' doesn't exist."
 	exit 1
@@ -52,7 +58,7 @@ if [ ! $? -eq 0 ] ; then
 	exit 1
 fi
 
-URL="${URL}/patches/?"
+URL="${URL}/${resource_type}/?"
 
 callcmd () # <patchwork id>
 {
-- 
2.25.1

[dpdk-ci] [PATCH 9/9] tools: filter new patchwork IDs by project name

[Ali Alnubani]

Different projects require different checks usually, this
patch modifies the script so that it requires a project's name
which it passes as a parameter with the API end point.

Example usage:
$ export TZ="Europe/Paris"
$ export MAINTAINERS_FILE_PATH=/path/to/dpdk/MAINTAINERS
$ ./tools/poll-pw series DPDK /path/to/last.txt \
    '/path/to/maintainers.py --command set_pw_delegate --type series $1'

Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
---
 tools/poll-pw | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/tools/poll-pw b/tools/poll-pw
index 6017146..67251cc 100755
--- a/tools/poll-pw
+++ b/tools/poll-pw
@@ -9,7 +9,7 @@ PAUSE_SECONDS=100
 
 print_usage () {
 	cat <<- END_OF_HELP
-	usage: $(basename $0) [OPTIONS] <series|patches> </path/to/last.txt> <command>
+	usage: $(basename $0) [OPTIONS] <series|patches> <project> </path/to/last.txt> <command>
 
 	Poll patchwork and call a command for each new patch/series id.
 	The first date to filter with is read from the specified file.
@@ -31,15 +31,16 @@ while getopts h arg ; do
 	esac
 done
 
-if [ $# -lt 3 ] ; then
+if [ $# -lt 4 ] ; then
 	printf 'missing argument(s)\n\n' >&2
 	print_usage >&2
 	exit 1
 fi
 shift $(($OPTIND - 1))
 resource_type=$1
-since_file=$2
-shift 2
+project=$2
+since_file=$3
+shift 3
 cmd=$*
 
 if ! curl -sfIo /dev/null $URL/$resource_type ; then
@@ -47,6 +48,12 @@ if ! curl -sfIo /dev/null $URL/$resource_type ; then
 	exit 1
 fi
 
+curl -s $URL/projects/ | jq '.[].name' | grep -qi "^\"${project}\"$"
+if [ ! $? -eq 0 ] ; then
+	echo "The project '$project' doesn't exist."
+	exit 1
+fi
+
 if [ ! -f "$since_file" ] ; then
 	echo "The file '$since_file' doesn't exist."
 	exit 1
@@ -58,7 +65,7 @@ if [ ! $? -eq 0 ] ; then
 	exit 1
 fi
 
-URL="${URL}/${resource_type}/?"
+URL="${URL}/${resource_type}/?project=${project}&"
 
 callcmd () # <patchwork id>
 {
-- 
2.25.1

[dpdk-ci] [PATCH v2 00/10] Automatic patchwork delegation

[alialnu]

From: Ali Alnubani <alialnu@nvidia.com>

This patchset adds support for automatic patch delegation.

The script 'guess_git_tree.py' was renamed to 'pw_maintainers_cli.py'
and now supports finding patch and series maintainers. It can also
delegate patches to them using git-pw.

The script 'poll-pw' was rewritten to fetch new api resources
by filtering with date and project. It now supports fetching both
patches and series.

Ali Alnubani (10):
  tools: rename guess_git_tree script
  tools: match by tree url instead of tree name
  tools: update script usage
  tools: add functionality for detecting tree maintainers
  tools: add functionality for setting pw delegates
  add git-pw to requirements file
  tools: filter new patchwork IDs by date
  tools: add support for fetching new series IDs
  tools: filter new patchwork IDs by project name
  tools: skip the IDs we already fetched

 requirements.txt                              |   3 +-
 tools/poll-pw                                 |  84 ++++++++----
 ...uess_git_tree.py => pw_maintainers_cli.py} | 121 ++++++++++++++----
 3 files changed, 161 insertions(+), 47 deletions(-)
 rename tools/{guess_git_tree.py => pw_maintainers_cli.py} (67%)

-- 
2.25.1

[dpdk-ci] [PATCH v2 01/10] tools: rename guess_git_tree script

[alialnu]

From: Ali Alnubani <alialnu@nvidia.com>

The name 'pw_maintainers_cli.py' will make more sense when
adding more operations to the script.

Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
---
Changes in v2:
- Changed the script's name to 'pw_maintainers_cli.py' instead of
  'maintainers.py' (Suggested by Juraj Linkes).

 tools/{guess_git_tree.py => pw_maintainers_cli.py} | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
 rename tools/{guess_git_tree.py => pw_maintainers_cli.py} (98%)

diff --git a/tools/guess_git_tree.py b/tools/pw_maintainers_cli.py
similarity index 98%
rename from tools/guess_git_tree.py
rename to tools/pw_maintainers_cli.py
index c9eef39..a31f605 100755
--- a/tools/guess_git_tree.py
+++ b/tools/pw_maintainers_cli.py
@@ -31,13 +31,13 @@ variables PW_{SERVER,PROJECT,TOKEN} should be set. If not, the script will try
 to load the git configurations pw.{server,project,token}.
 
 Example usage:
-    ./guess-git-tree.py --command list_trees_for_series 2054
-    ./guess-git-tree.py --command list_trees_for_patch 2054
+    ./pw_maintainers_cli.py --command list_trees_for_series 2054
+    ./pw_maintainers_cli.py --command list_trees_for_patch 2054
 
 Or if you want to use inside other scripts:
 
     import os
-    from guess_git_tree import (Maintainers, GitPW, Diff)
+    from pw_maintainers_cli import (Maintainers, GitPW, Diff)
     _git_pw = GitPW({
         'pw_server': os.environ.get('PW_SERVER'),
         'pw_project': os.environ.get('PW_PROJECT'),
-- 
2.25.1

[dpdk-ci] [PATCH v2 02/10] tools: match by tree url instead of tree name

[alialnu]

From: Ali Alnubani <alialnu@nvidia.com>

The tree url has more information. It can be used to
get the maintainer of a tree, and it includes the tree name anyway.

Also try to use named capture groups as much as possible.

Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
---
 tools/pw_maintainers_cli.py | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/tools/pw_maintainers_cli.py b/tools/pw_maintainers_cli.py
index a31f605..343e9f5 100755
--- a/tools/pw_maintainers_cli.py
+++ b/tools/pw_maintainers_cli.py
@@ -46,7 +46,8 @@ Or if you want to use inside other scripts:
     maintainers = Maintainers()
     patch_id = 52199
     files = Diff.find_filenames(_git_pw.api_get('patches', patch_id)['diff'])
-    tree = maintainers.get_tree(files)
+    tree_url = maintainers.get_tree(files)
+    tree_name = tree_url.split('/')[-1]
 """
 
 
@@ -116,9 +117,9 @@ class Diff(object):
 class Maintainers(object):
 
     file_regex = r'F:\s(.*)'
-    tree_regex = r'T: git:\/\/dpdk\.org(?:\/next)*\/(.*)'
+    tree_regex = r'T: (?P<url>git:\/\/dpdk\.org(?:\/next)*\/(?P<name>.*))'
     section_regex = r'([^\n]*)\n-+.*?(?=([^\n]*\n-+)|\Z)'
-    subsection_regex = r'[^\n](?:(?!\n{{2}}).)*?^F: {}(?:(?!\n{{2}}).)*'
+    subsection_regex = r'[^\n](?:(?!\n{{2}}).)*?^{}: {}$(?:(?!\n{{2}}).)*'
 
     def __init__(self):
         with open(MAINTAINERS_FILE_PATH) as fd:
@@ -151,8 +152,8 @@ class Maintainers(object):
             if _tree:
                 tree_list.append(_tree)
         tree = self.get_common_denominator(tree_list)
-        if tree == '':
-            tree = 'dpdk'
+        if not tree:
+            tree = 'git://dpdk.org/dpdk'
         return tree
 
     def _get_tree(self, filename):
@@ -180,7 +181,7 @@ class Maintainers(object):
 
         found_match = False
         # Find the block containing filename.
-        regex = self.subsection_regex.format(re.escape(matching_pattern))
+        regex = self.subsection_regex.format('F', re.escape(matching_pattern))
         subsection_match = re.findall(
                 regex,
                 self.maintainers_txt,
@@ -191,7 +192,7 @@ class Maintainers(object):
             tree_match = re.search(
                     self.tree_regex, subsection)
             if tree_match:
-                tree = tree_match.group(1)
+                tree = tree_match.group('url')
                 self.matched[matching_pattern] = tree
                 found_match = True
 
@@ -204,7 +205,7 @@ class Maintainers(object):
                             self.tree_regex,
                             section.group(0).split('\n\n')[0])
                     if tree_match:
-                        tree = tree_match.group(1)
+                        tree = tree_match.group('url')
 
         self.matched[matching_pattern] = tree
         return tree
@@ -228,8 +229,8 @@ class Maintainers(object):
             os.path.commonprefix(_tree_list).rstrip('-').replace(
                     'dpdk-next-net-virtio', 'dpdk-next-virtio')
         # There is no 'dpdk-next' named tree.
-        if common_prefix == 'dpdk-next':
-            common_prefix = 'dpdk'
+        if common_prefix.endswith('dpdk-next') or common_prefix.endswith('/'):
+            common_prefix = 'git://dpdk.org/dpdk'
         return common_prefix
 
 
@@ -289,4 +290,4 @@ if __name__ == '__main__':
     files = []
     for patch in patch_list:
         files += Diff.find_filenames(patch['diff'])
-    print(maintainers.get_tree(files))
+    print(maintainers.get_tree(files).split('/')[-1])
-- 
2.25.1

Re: [dpdk-ci] [PATCH v2 02/10] tools: match by tree url instead of tree name

[Thomas Monjalon]

21/09/2021 16:35, alialnu@nvidia.com:
> From: Ali Alnubani <alialnu@nvidia.com>
> 
> The tree url has more information. It can be used to

minor nit: s/url/URL/

> get the maintainer of a tree, and it includes the tree name anyway.

The only real difference is to have "next/" for trees having "next-" in their name.
And the URL prefix git://dpdk.org/ is added to provide a full URL.

> Also try to use named capture groups as much as possible.
> 
> Signed-off-by: Ali Alnubani <alialnu@nvidia.com>

Re: [dpdk-ci] [PATCH v2 02/10] tools: match by tree url instead of tree name

[Ali Alnubani]

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Thursday, September 30, 2021 11:01 AM
> To: Ali Alnubani <alialnu@nvidia.com>
> Cc: ci@dpdk.org; jerinj@marvell.com; ferruh.yigit@intel.com;
> david.marchand@redhat.com; juraj.linkes@pantheon.tech
> Subject: Re: [PATCH v2 02/10] tools: match by tree url instead of tree name
> 
> 21/09/2021 16:35, alialnu@nvidia.com:
> > From: Ali Alnubani <alialnu@nvidia.com>
> >
> > The tree url has more information. It can be used to
> 
> minor nit: s/url/URL/
> 
> > get the maintainer of a tree, and it includes the tree name anyway.
> 
> The only real difference is to have "next/" for trees having "next-" in their
> name.
> And the URL prefix git://dpdk.org/ is added to provide a full URL.

Updated the commits description in v3.

> 
> > Also try to use named capture groups as much as possible.
> >
> > Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
> 
> 
> 

[dpdk-ci] [PATCH v2 03/10] tools: update script usage

[alialnu]

From: Ali Alnubani <alialnu@nvidia.com>

The resource type (whether it's a patch or a series) is now
specified by the new argument --type.
Both commands: list_trees_for_patch & list_trees_for_series
are replaced by the positional arg 'list_trees', and the type must
be always set.

This makes adding more operations require less arguments.

Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
---
 tools/pw_maintainers_cli.py | 32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/tools/pw_maintainers_cli.py b/tools/pw_maintainers_cli.py
index 343e9f5..d4c0056 100755
--- a/tools/pw_maintainers_cli.py
+++ b/tools/pw_maintainers_cli.py
@@ -31,8 +31,8 @@ variables PW_{SERVER,PROJECT,TOKEN} should be set. If not, the script will try
 to load the git configurations pw.{server,project,token}.
 
 Example usage:
-    ./pw_maintainers_cli.py --command list_trees_for_series 2054
-    ./pw_maintainers_cli.py --command list_trees_for_patch 2054
+    ./pw_maintainers_cli.py --type series list_trees 2054
+    ./pw_maintainers_cli.py --type patch list_trees 2054
 
 Or if you want to use inside other scripts:
 
@@ -238,14 +238,14 @@ if __name__ == '__main__':
     """Main procedure."""
     parser = argparse.ArgumentParser()
     git_pw_conf_parser = parser.add_argument_group('git-pw configurations')
-    options_parser = parser.add_argument_group('optional arguments')
+    required_args_parser = parser.add_argument_group('required arguments')
 
-    options_parser.add_argument(
-            '--command',
+    required_args_parser.add_argument(
+            '--type',
             choices=(
-                'list_trees_for_patch',
-                'list_trees_for_series'),
-            required=True, help='Command to perform')
+                'patch',
+                'series'),
+            required=True, help='Resource type.')
 
     git_pw_conf_parser.add_argument(
             '--pw_server', type=str,
@@ -262,12 +262,18 @@ if __name__ == '__main__':
             default=os.environ.get('PW_TOKEN', utils.git_config('pw.token')),
             help='Authentication token')
 
+    parser.add_argument(
+            'command',
+            choices=[
+                'list_trees'],
+            help='Command to perform')
     parser.add_argument(
             'id', type=int, help='patch/series id')
 
     args = parser.parse_args()
 
     command = args.command
+    resource_type = args.type
     _id = args.id
 
     # Pass the needed configurations to git-pw.
@@ -279,9 +285,9 @@ if __name__ == '__main__':
     maintainers = Maintainers()
 
     patch_list = []
-    if command == 'list_trees_for_patch':
+    if resource_type == 'patch':
         patch_list.append(_git_pw.api_get('patches', _id))
-    elif command == 'list_trees_for_series':
+    else:
         series = _git_pw.api_get('series', _id)
         patch_list = [
                 _git_pw.api_get('patches', patch['id'])
@@ -290,4 +296,8 @@ if __name__ == '__main__':
     files = []
     for patch in patch_list:
         files += Diff.find_filenames(patch['diff'])
-    print(maintainers.get_tree(files).split('/')[-1])
+
+    tree = maintainers.get_tree(files)
+
+    if command == 'list_trees':
+        print(tree.split('/')[-1])
-- 
2.25.1

Re: [dpdk-ci] [PATCH v2 03/10] tools: update script usage

[Thomas Monjalon]

21/09/2021 16:35, alialnu@nvidia.com:
> From: Ali Alnubani <alialnu@nvidia.com>
> 
> The resource type (whether it's a patch or a series) is now
> specified by the new argument --type.
> Both commands: list_trees_for_patch & list_trees_for_series
> are replaced by the positional arg 'list_trees', and the type must
> be always set.
> 
> This makes adding more operations require less arguments.
> 
> Signed-off-by: Ali Alnubani <alialnu@nvidia.com>

Acked-by: Thomas Monjalon <thomas@monjalon.net>

[dpdk-ci] [PATCH v2 04/10] tools: add functionality for detecting tree maintainers

[alialnu]

From: Ali Alnubani <alialnu@nvidia.com>

Detecting a maintainer works by searching the
'General Project Administration' section for subsections
containing the provided tree, and then returning the maintainers
specified in that subsection.

Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
---
 tools/pw_maintainers_cli.py | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/tools/pw_maintainers_cli.py b/tools/pw_maintainers_cli.py
index d4c0056..9b77e77 100755
--- a/tools/pw_maintainers_cli.py
+++ b/tools/pw_maintainers_cli.py
@@ -33,6 +33,7 @@ to load the git configurations pw.{server,project,token}.
 Example usage:
     ./pw_maintainers_cli.py --type series list_trees 2054
     ./pw_maintainers_cli.py --type patch list_trees 2054
+    ./pw_maintainers_cli.py --type patch list_maintainers 2054
 
 Or if you want to use inside other scripts:
 
@@ -48,6 +49,7 @@ Or if you want to use inside other scripts:
     files = Diff.find_filenames(_git_pw.api_get('patches', patch_id)['diff'])
     tree_url = maintainers.get_tree(files)
     tree_name = tree_url.split('/')[-1]
+    maintainers = maintainers.get_maintainers(tree_url)
 """
 
 
@@ -118,6 +120,7 @@ class Maintainers(object):
 
     file_regex = r'F:\s(.*)'
     tree_regex = r'T: (?P<url>git:\/\/dpdk\.org(?:\/next)*\/(?P<name>.*))'
+    maintainer_regex = r'M:\s(.*)'
     section_regex = r'([^\n]*)\n-+.*?(?=([^\n]*\n-+)|\Z)'
     subsection_regex = r'[^\n](?:(?!\n{{2}}).)*?^{}: {}$(?:(?!\n{{2}}).)*'
 
@@ -141,6 +144,26 @@ class Maintainers(object):
         # Save already matched patterns.
         self.matched = {}
 
+    def get_maintainers(self, tree):
+        """
+        Return a list of a tree's maintainers."""
+        maintainers = []
+        for section in self.sections:
+            if section.group(1) == 'General Project Administration':
+                # Find the block containing the tree.
+                regex = self.subsection_regex.format('T', re.escape(tree))
+                subsection_match = re.findall(
+                        regex,
+                        section.group(0),
+                        re.DOTALL | re.MULTILINE)
+                if len(subsection_match):
+                    subsection = subsection_match[-1]
+                    # Look for maintainers
+                    maintainers = re.findall(
+                            self.maintainer_regex, subsection)
+                    return maintainers
+                break
+
     def get_tree(self, files):
         """
         Return a git tree that matches a list of files."""
@@ -265,7 +288,7 @@ if __name__ == '__main__':
     parser.add_argument(
             'command',
             choices=[
-                'list_trees'],
+                'list_trees', 'list_maintainers'],
             help='Command to perform')
     parser.add_argument(
             'id', type=int, help='patch/series id')
@@ -301,3 +324,5 @@ if __name__ == '__main__':
 
     if command == 'list_trees':
         print(tree.split('/')[-1])
+    elif command == 'list_maintainers':
+        print(*maintainers.get_maintainers(tree), sep='\n')
-- 
2.25.1

Re: [dpdk-ci] [PATCH v2 04/10] tools: add functionality for detecting tree maintainers

[Thomas Monjalon]

21/09/2021 16:35, alialnu@nvidia.com:
> From: Ali Alnubani <alialnu@nvidia.com>
> 
> Detecting a maintainer works by searching the
> 'General Project Administration' section for subsections
> containing the provided tree, and then returning the maintainers
> specified in that subsection.
> 
> Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
> ---
> @@ -118,6 +120,7 @@ class Maintainers(object):
>  
>      file_regex = r'F:\s(.*)'
>      tree_regex = r'T: (?P<url>git:\/\/dpdk\.org(?:\/next)*\/(?P<name>.*))'
> +    maintainer_regex = r'M:\s(.*)'
>      section_regex = r'([^\n]*)\n-+.*?(?=([^\n]*\n-+)|\Z)'
>      subsection_regex = r'[^\n](?:(?!\n{{2}}).)*?^{}: {}$(?:(?!\n{{2}}).)*'
>  
> @@ -141,6 +144,26 @@ class Maintainers(object):
>          # Save already matched patterns.
>          self.matched = {}
>  
> +    def get_maintainers(self, tree):
> +        """
> +        Return a list of a tree's maintainers."""
> +        maintainers = []
> +        for section in self.sections:
> +            if section.group(1) == 'General Project Administration':

Should this constant string be declared above with other regex?

> +                # Find the block containing the tree.
> +                regex = self.subsection_regex.format('T', re.escape(tree))
> +                subsection_match = re.findall(
> +                        regex,
> +                        section.group(0),
> +                        re.DOTALL | re.MULTILINE)
> +                if len(subsection_match):
> +                    subsection = subsection_match[-1]
> +                    # Look for maintainers
> +                    maintainers = re.findall(
> +                            self.maintainer_regex, subsection)
> +                    return maintainers
> +                break

[dpdk-ci] [PATCH v2 05/10] tools: add functionality for setting pw delegates

[alialnu]

From: Ali Alnubani <alialnu@nvidia.com>

A new command was added to set patch delegates in Patchwork
based on the emails found in DPDK's MAINTAINERS file.

Example usage:
  $ export MAINTAINERS_FILE_PATH=/path/to/dpdk/MAINTAINERS
  $ ./pw_maintainers_cli.py --type series set_pw_delegate SERIES_ID

Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
---
 tools/pw_maintainers_cli.py | 47 ++++++++++++++++++++++++++++++++++---
 1 file changed, 44 insertions(+), 3 deletions(-)

diff --git a/tools/pw_maintainers_cli.py b/tools/pw_maintainers_cli.py
index 9b77e77..e68f049 100755
--- a/tools/pw_maintainers_cli.py
+++ b/tools/pw_maintainers_cli.py
@@ -14,6 +14,7 @@ from requests.exceptions import HTTPError
 from git_pw import config
 from git_pw import api
 from git_pw import utils
+from git_pw import patch as git_pw_patch
 
 """
 Description:
@@ -84,6 +85,31 @@ class GitPW(object):
             else:
                 raise
 
+    def set_delegate(self, patch_list, delegate):
+        """Set the delegate for a patch.
+        Only tries to set a delegate for patches that don't have
+        one set already.
+
+        Reference:
+        https://github.com/getpatchwork/git-pw/blob/76b79097dc0a57c89b45dd53d9cacb7ff7b31bb2/git_pw/patch.py#L167
+        """
+        users = api.index('users', [('q', delegate)])
+        if len(users) != 1:
+            # Zero or multiple users found
+            print('Cannot choose a Patchwork user to delegate to from '
+                  'user list ({}). Skipping..'.format(users))
+            return
+        for patch in patch_list:
+            if patch['delegate']:
+                print('Patch {} is already delegated to {}. '
+                      'Skipping..'.format(
+                          patch['id'], patch['delegate']['email']))
+                continue
+            print("Delegating patch {} to {}.".format(
+                patch['id'], users[0]['email']))
+            _ = api.update(
+                    'patches', patch['id'], [('delegate', users[0]['id'])])
+
 
 class Diff(object):
 
@@ -288,7 +314,7 @@ if __name__ == '__main__':
     parser.add_argument(
             'command',
             choices=[
-                'list_trees', 'list_maintainers'],
+                'list_trees', 'list_maintainers', 'set_pw_delegate'],
             help='Command to perform')
     parser.add_argument(
             'id', type=int, help='patch/series id')
@@ -324,5 +350,20 @@ if __name__ == '__main__':
 
     if command == 'list_trees':
         print(tree.split('/')[-1])
-    elif command == 'list_maintainers':
-        print(*maintainers.get_maintainers(tree), sep='\n')
+    if command in ['list_maintainers', 'set_pw_delegate']:
+        maintainer_list = maintainers.get_maintainers(tree)
+        if command == 'list_maintainers':
+            print(*maintainer_list, sep='\n')
+        elif command == 'set_pw_delegate':
+            if len(maintainer_list) > 0:
+                # Get the email of the first maintainer in the list.
+                try:
+                    delegate = re.match(
+                            r".*\<(?P<email>.*)\>",
+                            maintainer_list[0]).group('email')
+                except AttributeError:
+                    print("Unexpected format: '{}'".format(maintainer_list[0]))
+                    sys.exit(1)
+                _git_pw.set_delegate(patch_list, delegate)
+            else:
+                print('No maintainers found. Not setting a delegate.')
-- 
2.25.1

Re: [dpdk-ci] [PATCH v2 05/10] tools: add functionality for setting pw delegates

[Thomas Monjalon]

21/09/2021 16:35, alialnu@nvidia.com:
> From: Ali Alnubani <alialnu@nvidia.com>
> 
> A new command was added to set patch delegates in Patchwork
> based on the emails found in DPDK's MAINTAINERS file.
> 
> Example usage:
>   $ export MAINTAINERS_FILE_PATH=/path/to/dpdk/MAINTAINERS
>   $ ./pw_maintainers_cli.py --type series set_pw_delegate SERIES_ID
> 
> Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
> ---
> +    def set_delegate(self, patch_list, delegate):
> +        """Set the delegate for a patch.
> +        Only tries to set a delegate for patches that don't have
> +        one set already.

I'm not sure we should skip patches which are already delegated.
If we use the command to explicitly delegate the patch,
we should do it, right?

The skip logic may be implemented at a higher level in the CI.

> +        Reference:
> +        https://github.com/getpatchwork/git-pw/blob/76b79097dc0a57c89b45dd53d9cacb7ff7b31bb2/git_pw/patch.py#L167
> +        """
> +        users = api.index('users', [('q', delegate)])
> +        if len(users) != 1:
> +            # Zero or multiple users found
> +            print('Cannot choose a Patchwork user to delegate to from '
> +                  'user list ({}). Skipping..'.format(users))
> +            return
> +        for patch in patch_list:
> +            if patch['delegate']:
> +                print('Patch {} is already delegated to {}. '
> +                      'Skipping..'.format(
> +                          patch['id'], patch['delegate']['email']))
> +                continue
> +            print("Delegating patch {} to {}.".format(
> +                patch['id'], users[0]['email']))
> +            _ = api.update(
> +                    'patches', patch['id'], [('delegate', users[0]['id'])])

Re: [dpdk-ci] [PATCH v2 05/10] tools: add functionality for setting pw delegates

[Ali Alnubani]

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Thursday, September 30, 2021 12:15 PM
> To: Ali Alnubani <alialnu@nvidia.com>
> Cc: ci@dpdk.org; jerinj@marvell.com; ferruh.yigit@intel.com;
> david.marchand@redhat.com; juraj.linkes@pantheon.tech
> Subject: Re: [PATCH v2 05/10] tools: add functionality for setting pw
> delegates
> 
> 21/09/2021 16:35, alialnu@nvidia.com:
> > From: Ali Alnubani <alialnu@nvidia.com>
> >
> > A new command was added to set patch delegates in Patchwork
> > based on the emails found in DPDK's MAINTAINERS file.
> >
> > Example usage:
> >   $ export MAINTAINERS_FILE_PATH=/path/to/dpdk/MAINTAINERS
> >   $ ./pw_maintainers_cli.py --type series set_pw_delegate SERIES_ID
> >
> > Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
> > ---
> > +    def set_delegate(self, patch_list, delegate):
> > +        """Set the delegate for a patch.
> > +        Only tries to set a delegate for patches that don't have
> > +        one set already.
> 
> I'm not sure we should skip patches which are already delegated.
> If we use the command to explicitly delegate the patch,
> we should do it, right?
> 
> The skip logic may be implemented at a higher level in the CI.

I added an arg (--force_set_delegate) to force overriding delegates in v3.

> 
> > +        Reference:
> > +        https://github.com/getpatchwork/git-
> pw/blob/76b79097dc0a57c89b45dd53d9cacb7ff7b31bb2/git_pw/patch.py#L1
> 67
> > +        """
> > +        users = api.index('users', [('q', delegate)])
> > +        if len(users) != 1:
> > +            # Zero or multiple users found
> > +            print('Cannot choose a Patchwork user to delegate to from '
> > +                  'user list ({}). Skipping..'.format(users))
> > +            return
> > +        for patch in patch_list:
> > +            if patch['delegate']:
> > +                print('Patch {} is already delegated to {}. '
> > +                      'Skipping..'.format(
> > +                          patch['id'], patch['delegate']['email']))
> > +                continue
> > +            print("Delegating patch {} to {}.".format(
> > +                patch['id'], users[0]['email']))
> > +            _ = api.update(
> > +                    'patches', patch['id'], [('delegate', users[0]['id'])])
> 
> 

Re: [dpdk-ci] [PATCH v2 05/10] tools: add functionality for setting pw delegates

[Thomas Monjalon]

18/10/2021 09:48, Ali Alnubani:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 21/09/2021 16:35, alialnu@nvidia.com:
> > > From: Ali Alnubani <alialnu@nvidia.com>
> > >
> > > A new command was added to set patch delegates in Patchwork
> > > based on the emails found in DPDK's MAINTAINERS file.
> > >
> > > Example usage:
> > >   $ export MAINTAINERS_FILE_PATH=/path/to/dpdk/MAINTAINERS
> > >   $ ./pw_maintainers_cli.py --type series set_pw_delegate SERIES_ID
> > >
> > > Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
> > > ---
> > > +    def set_delegate(self, patch_list, delegate):
> > > +        """Set the delegate for a patch.
> > > +        Only tries to set a delegate for patches that don't have
> > > +        one set already.
> > 
> > I'm not sure we should skip patches which are already delegated.
> > If we use the command to explicitly delegate the patch,
> > we should do it, right?
> > 
> > The skip logic may be implemented at a higher level in the CI.
> 
> I added an arg (--force_set_delegate) to force overriding delegates in v3.

Given the command is to set delegate, the force looks strange to me.
Wouldn't it be more logical to add an option to skip already delegated patches?

Re: [dpdk-ci] [PATCH v2 05/10] tools: add functionality for setting pw delegates

[Ali Alnubani]

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Tuesday, October 26, 2021 5:08 PM
> To: Ali Alnubani <alialnu@nvidia.com>
> Cc: ci@dpdk.org; jerinj@marvell.com; ferruh.yigit@intel.com;
> david.marchand@redhat.com; juraj.linkes@pantheon.tech
> Subject: Re: [PATCH v2 05/10] tools: add functionality for setting pw
> delegates
> 
> 18/10/2021 09:48, Ali Alnubani:
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 21/09/2021 16:35, alialnu@nvidia.com:
> > > > From: Ali Alnubani <alialnu@nvidia.com>
> > > >
> > > > A new command was added to set patch delegates in Patchwork
> > > > based on the emails found in DPDK's MAINTAINERS file.
> > > >
> > > > Example usage:
> > > >   $ export MAINTAINERS_FILE_PATH=/path/to/dpdk/MAINTAINERS
> > > >   $ ./pw_maintainers_cli.py --type series set_pw_delegate SERIES_ID
> > > >
> > > > Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
> > > > ---
> > > > +    def set_delegate(self, patch_list, delegate):
> > > > +        """Set the delegate for a patch.
> > > > +        Only tries to set a delegate for patches that don't have
> > > > +        one set already.
> > >
> > > I'm not sure we should skip patches which are already delegated.
> > > If we use the command to explicitly delegate the patch,
> > > we should do it, right?
> > >
> > > The skip logic may be implemented at a higher level in the CI.
> >
> > I added an arg (--force_set_delegate) to force overriding delegates in v3.
> 
> Given the command is to set delegate, the force looks strange to me.
> Wouldn't it be more logical to add an option to skip already delegated
> patches?

Isn't it safer not to force overriding the delegate by default?
Users can send patches with the header "X-Patchwork-Delegate" to set a specific delegate. Maintainers might want to set another user as the delegate when there are multiple delegates in the MAINTAINERS file for a patch. Should we override these by default?

Re: [dpdk-ci] [PATCH v2 05/10] tools: add functionality for setting pw delegates

[Thomas Monjalon]

04/11/2021 17:48, Ali Alnubani:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 18/10/2021 09:48, Ali Alnubani:
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > 21/09/2021 16:35, alialnu@nvidia.com:
> > > > > From: Ali Alnubani <alialnu@nvidia.com>
> > > > >
> > > > > A new command was added to set patch delegates in Patchwork
> > > > > based on the emails found in DPDK's MAINTAINERS file.
> > > > >
> > > > > Example usage:
> > > > >   $ export MAINTAINERS_FILE_PATH=/path/to/dpdk/MAINTAINERS
> > > > >   $ ./pw_maintainers_cli.py --type series set_pw_delegate SERIES_ID
> > > > >
> > > > > Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
> > > > > ---
> > > > > +    def set_delegate(self, patch_list, delegate):
> > > > > +        """Set the delegate for a patch.
> > > > > +        Only tries to set a delegate for patches that don't have
> > > > > +        one set already.
> > > >
> > > > I'm not sure we should skip patches which are already delegated.
> > > > If we use the command to explicitly delegate the patch,
> > > > we should do it, right?
> > > >
> > > > The skip logic may be implemented at a higher level in the CI.
> > >
> > > I added an arg (--force_set_delegate) to force overriding delegates in v3.
> > 
> > Given the command is to set delegate, the force looks strange to me.
> > Wouldn't it be more logical to add an option to skip already delegated
> > patches?
> 
> Isn't it safer not to force overriding the delegate by default?
> Users can send patches with the header "X-Patchwork-Delegate" to set a specific delegate. Maintainers might want to set another user as the delegate when there are multiple delegates in the MAINTAINERS file for a patch. Should we override these by default?

These considerations don't contradict with the option name.
If we have the option --skip-delegated (or --only-non-delegated),
we can always use this option in the automatic run.

But for a manual run, I expect the script to delegate a patch
the user explicitly wants to set, without requiring a "force" option.

Re: [dpdk-ci] [PATCH v2 05/10] tools: add functionality for setting pw delegates

[Ali Alnubani]

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Thursday, November 4, 2021 8:16 PM
> To: Ali Alnubani <alialnu@nvidia.com>
> Cc: ci@dpdk.org; jerinj@marvell.com; ferruh.yigit@intel.com;
> david.marchand@redhat.com; juraj.linkes@pantheon.tech
> Subject: Re: [PATCH v2 05/10] tools: add functionality for setting pw
> delegates
> 
> 04/11/2021 17:48, Ali Alnubani:
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 18/10/2021 09:48, Ali Alnubani:
> > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > 21/09/2021 16:35, alialnu@nvidia.com:
> > > > > > From: Ali Alnubani <alialnu@nvidia.com>
> > > > > >
> > > > > > A new command was added to set patch delegates in Patchwork
> > > > > > based on the emails found in DPDK's MAINTAINERS file.
> > > > > >
> > > > > > Example usage:
> > > > > >   $ export MAINTAINERS_FILE_PATH=/path/to/dpdk/MAINTAINERS
> > > > > >   $ ./pw_maintainers_cli.py --type series set_pw_delegate
> SERIES_ID
> > > > > >
> > > > > > Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
> > > > > > ---
> > > > > > +    def set_delegate(self, patch_list, delegate):
> > > > > > +        """Set the delegate for a patch.
> > > > > > +        Only tries to set a delegate for patches that don't have
> > > > > > +        one set already.
> > > > >
> > > > > I'm not sure we should skip patches which are already delegated.
> > > > > If we use the command to explicitly delegate the patch,
> > > > > we should do it, right?
> > > > >
> > > > > The skip logic may be implemented at a higher level in the CI.
> > > >
> > > > I added an arg (--force_set_delegate) to force overriding delegates in
> v3.
> > >
> > > Given the command is to set delegate, the force looks strange to me.
> > > Wouldn't it be more logical to add an option to skip already delegated
> > > patches?
> >
> > Isn't it safer not to force overriding the delegate by default?
> > Users can send patches with the header "X-Patchwork-Delegate" to set a
> specific delegate. Maintainers might want to set another user as the
> delegate when there are multiple delegates in the MAINTAINERS file for a
> patch. Should we override these by default?
> 
> These considerations don't contradict with the option name.
> If we have the option --skip-delegated (or --only-non-delegated),
> we can always use this option in the automatic run.
> 
> But for a manual run, I expect the script to delegate a patch
> the user explicitly wants to set, without requiring a "force" option.

Thanks for the feedback, updated in v4 and added a --skip-delegated arg instead:
https://inbox.dpdk.org/ci/20211108062815.7614-6-alialnu@nvidia.com/T/#u

[dpdk-ci] [PATCH v2 06/10] add git-pw to requirements file

[alialnu]

From: Ali Alnubani <alialnu@nvidia.com>

The module has always been required by tools/maintainers.py (previously
named tools/guess_git_tree.py).

Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
---
 requirements.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/requirements.txt b/requirements.txt
index f20067d..f2a6844 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1 +1,2 @@
-whatthepatch==1.0.2
\ No newline at end of file
+git-pw==2.1.0
+whatthepatch==1.0.2
-- 
2.25.1

[dpdk-ci] [PATCH v2 07/10] tools: filter new patchwork IDs by date

[alialnu]

From: Ali Alnubani <alialnu@nvidia.com>

API resource IDs are guaranteed to be unique, but they aren't guaranteed
to have no gaps, for example, the following series IDs are
nonexistent: 16157, 17181, 18235.
Filtering by the date since the last check is necessary to later
add support for fetching new series IDs in addition to patch IDs.

Instead of requiring a file that contains the next patch ID,
a file containing the timestamp of the last time the API was fetched
is now used.
Each time the API is fetched for new patches, the timestamp
in the file gets updated, and the script sleeps an amount of time specified
by PAUSE_SECONDS before attempting to fetch new resources again.

The pause amount between each poll attempt is still 100 seconds.

Setting the env variable 'TZ' might be necessary if your timezone
doesn't match the server's timezone.

The package jq (Command-line JSON processor) is now required by the
script.

Example usage:
$ export TZ="Europe/Paris"
$ export MAINTAINERS_FILE_PATH=/path/to/dpdk/MAINTAINERS
$ ./tools/poll-pw /path/to/last.txt \
    '/path/to/pw_maintainers_cli.py --type patch set_pw_delegate $1'

Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
---
Changes in v2:
- Removed an unnecessary 'break' statement in the for loop.

 tools/poll-pw | 56 ++++++++++++++++++++++++++++++++-------------------
 1 file changed, 35 insertions(+), 21 deletions(-)

diff --git a/tools/poll-pw b/tools/poll-pw
index bdf860a..6ac8a61 100755
--- a/tools/poll-pw
+++ b/tools/poll-pw
@@ -1,58 +1,72 @@
-#! /bin/sh -e
+#! /bin/sh
 
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright 2017 6WIND S.A.
 # Copyright 2018 Mellanox Technologies, Ltd
 
 URL=http://patches.dpdk.org/api
+PAUSE_SECONDS=100
 
 print_usage () {
 	cat <<- END_OF_HELP
-	usage: $(basename $0) <counter> <command>
+	usage: $(basename $0) [OPTIONS] </path/to/last.txt> <command>
 
 	Poll patchwork and call command for each patch.
-	The first patchwork id to be checked is read from the counter file.
-	The command should use '$1' to be evaluated as patchwork id.
-	When a patch is found and the command is successful,
-	then the counter is incremented.
+	The first date to filter with is read from the specified file.
+	The command should use '$1' to be evaluated as the patch id.
+	The date in the specifed file is updated after each pull.
 	END_OF_HELP
 }
 
+which jq >/dev/null 2>&1
+if [ ! $? -eq 0 ] ; then
+	echo "The command 'jq' doesn't exist, please install it."
+	exit 1
+fi
+
 while getopts h arg ; do
 	case $arg in
 		h ) print_usage ; exit 0 ;;
 		? ) print_usage >&2 ; exit 1 ;;
 	esac
 done
+
 if [ $# -lt 2 ] ; then
-	printf 'missing argument\n\n' >&2
+	printf 'missing argument(s)\n\n' >&2
 	print_usage >&2
 	exit 1
 fi
 shift $(($OPTIND - 1))
-counter=$1
+since_file=$1
 shift
 cmd=$*
 
+if [ ! -f "$since_file" ] ; then
+	echo "The file '$since_file' doesn't exist."
+	exit 1
+fi
+
+date -d "$(cat $since_file | tr '\n' ' ')" >/dev/null 2>&1
+if [ ! $? -eq 0 ] ; then
+	echo "The file '$since_file' doesn't contain a valid date format."
+	exit 1
+fi
+
+URL="${URL}/patches/?"
+
 callcmd () # <patchwork id>
 {
 	eval $cmd
 }
 
-checkid () # <patchwork id>
-{
-	curl -sfIo /dev/null $URL/patches/$1/ ||
-	curl -sfIo /dev/null $URL/covers/$1/
-}
-
-pwid=$(cat $counter)
+set -e
 while true ; do
-	# process all recent patches
-	while checkid $pwid ; do
-		callcmd $pwid || break
-		pwid=$(($pwid + 1))
-		echo $pwid >$counter
+	date_now=$(date '+%FT%T')
+	since=$(date -d $(cat $since_file | tr '\n' ' ') '+%FT%T')
+	for id in $(curl -s "${URL}since=${since}" | jq '.[].id') ; do
+		callcmd $id
 	done
+	echo -n $date_now >$since_file
 	# pause before next check
-	sleep 100
+	sleep $PAUSE_SECONDS
 done
-- 
2.25.1

Re: [dpdk-ci] [PATCH v2 07/10] tools: filter new patchwork IDs by date

[Ali Alnubani]

> -----Original Message-----
> From: ci <ci-bounces@dpdk.org> On Behalf Of alialnu@oss.nvidia.com
> Sent: Tuesday, September 21, 2021 5:36 PM
> To: ci@dpdk.org
> Cc: NBU-Contact-Thomas Monjalon <thomas@monjalon.net>;
> jerinj@marvell.com; ferruh.yigit@intel.com; david.marchand@redhat.com;
> juraj.linkes@pantheon.tech
> Subject: [dpdk-ci] [PATCH v2 07/10] tools: filter new patchwork IDs by date
> 
> From: Ali Alnubani <alialnu@nvidia.com>
> 
> API resource IDs are guaranteed to be unique, but they aren't guaranteed
> to have no gaps, for example, the following series IDs are
> nonexistent: 16157, 17181, 18235.
> Filtering by the date since the last check is necessary to later
> add support for fetching new series IDs in addition to patch IDs.
> 
> Instead of requiring a file that contains the next patch ID,
> a file containing the timestamp of the last time the API was fetched
> is now used.
> Each time the API is fetched for new patches, the timestamp
> in the file gets updated, and the script sleeps an amount of time specified
> by PAUSE_SECONDS before attempting to fetch new resources again.
> 
> The pause amount between each poll attempt is still 100 seconds.
> 
> Setting the env variable 'TZ' might be necessary if your timezone
> doesn't match the server's timezone.
> 
> The package jq (Command-line JSON processor) is now required by the
> script.
> 
> Example usage:
> $ export TZ="Europe/Paris"
> $ export MAINTAINERS_FILE_PATH=/path/to/dpdk/MAINTAINERS
> $ ./tools/poll-pw /path/to/last.txt \
>     '/path/to/pw_maintainers_cli.py --type patch set_pw_delegate $1'
> 
> Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
> ---

I have mistakenly assumed that the date field I'm filtering with is the date Patchwork creates the ID, when in fact it's the date the patch was created on. I'll need to filter the /events end point instead.

[dpdk-ci] [PATCH v2 08/10] tools: add support for fetching new series IDs

[alialnu]

From: Ali Alnubani <alialnu@nvidia.com>

The script can now fetch new series IDs the same way
it fetches new patch IDs.

Example usage:
$ export TZ="Europe/Paris"
$ export MAINTAINERS_FILE_PATH=/path/to/dpdk/MAINTAINERS
$ ./tools/poll-pw series /path/to/last.txt \
    '/path/to/pw_maintainers_cli.py --type series set_pw_delegate $1'

Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
---
 tools/poll-pw | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/tools/poll-pw b/tools/poll-pw
index 6ac8a61..bf1af70 100755
--- a/tools/poll-pw
+++ b/tools/poll-pw
@@ -9,11 +9,11 @@ PAUSE_SECONDS=100
 
 print_usage () {
 	cat <<- END_OF_HELP
-	usage: $(basename $0) [OPTIONS] </path/to/last.txt> <command>
+	usage: $(basename $0) [OPTIONS] <series|patches> </path/to/last.txt> <command>
 
-	Poll patchwork and call command for each patch.
+	Poll patchwork and call a command for each new patch/series id.
 	The first date to filter with is read from the specified file.
-	The command should use '$1' to be evaluated as the patch id.
+	The command should use '$1' to be evaluated as the patch/series id.
 	The date in the specifed file is updated after each pull.
 	END_OF_HELP
 }
@@ -31,16 +31,22 @@ while getopts h arg ; do
 	esac
 done
 
-if [ $# -lt 2 ] ; then
+if [ $# -lt 3 ] ; then
 	printf 'missing argument(s)\n\n' >&2
 	print_usage >&2
 	exit 1
 fi
 shift $(($OPTIND - 1))
-since_file=$1
-shift
+resource_type=$1
+since_file=$2
+shift 2
 cmd=$*
 
+if ! curl -sfIo /dev/null $URL/$resource_type ; then
+	echo "API endpoint '$URL/$resource_type' doesn't exist or inaccessible."
+	exit 1
+fi
+
 if [ ! -f "$since_file" ] ; then
 	echo "The file '$since_file' doesn't exist."
 	exit 1
@@ -52,7 +58,7 @@ if [ ! $? -eq 0 ] ; then
 	exit 1
 fi
 
-URL="${URL}/patches/?"
+URL="${URL}/${resource_type}/?"
 
 callcmd () # <patchwork id>
 {
-- 
2.25.1

Re: [dpdk-ci] [PATCH v2 08/10] tools: add support for fetching new series IDs

[Thomas Monjalon]

21/09/2021 16:35, alialnu@nvidia.com:
> From: Ali Alnubani <alialnu@nvidia.com>
> 
> The script can now fetch new series IDs the same way
> it fetches new patch IDs.

The title can be simply "tools: support fetching series"

[dpdk-ci] [PATCH v2 09/10] tools: filter new patchwork IDs by project name

[alialnu]

From: Ali Alnubani <alialnu@nvidia.com>

Different projects require different checks usually, this
patch modifies the script so that it requires a project's name
which it passes as a parameter with the API end point.

Example usage:
$ export TZ="Europe/Paris"
$ export MAINTAINERS_FILE_PATH=/path/to/dpdk/MAINTAINERS
$ ./tools/poll-pw series DPDK /path/to/last.txt \
    '/path/to/pw_maintainers_cli.py --type series set_pw_delegate $1'

Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
---
 tools/poll-pw | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/tools/poll-pw b/tools/poll-pw
index bf1af70..e197a91 100755
--- a/tools/poll-pw
+++ b/tools/poll-pw
@@ -9,7 +9,7 @@ PAUSE_SECONDS=100
 
 print_usage () {
 	cat <<- END_OF_HELP
-	usage: $(basename $0) [OPTIONS] <series|patches> </path/to/last.txt> <command>
+	usage: $(basename $0) [OPTIONS] <series|patches> <project> </path/to/last.txt> <command>
 
 	Poll patchwork and call a command for each new patch/series id.
 	The first date to filter with is read from the specified file.
@@ -31,15 +31,16 @@ while getopts h arg ; do
 	esac
 done
 
-if [ $# -lt 3 ] ; then
+if [ $# -lt 4 ] ; then
 	printf 'missing argument(s)\n\n' >&2
 	print_usage >&2
 	exit 1
 fi
 shift $(($OPTIND - 1))
 resource_type=$1
-since_file=$2
-shift 2
+project=$2
+since_file=$3
+shift 3
 cmd=$*
 
 if ! curl -sfIo /dev/null $URL/$resource_type ; then
@@ -47,6 +48,12 @@ if ! curl -sfIo /dev/null $URL/$resource_type ; then
 	exit 1
 fi
 
+curl -s $URL/projects/ | jq '.[].name' | grep -qi "^\"${project}\"$"
+if [ ! $? -eq 0 ] ; then
+	echo "The project '$project' doesn't exist."
+	exit 1
+fi
+
 if [ ! -f "$since_file" ] ; then
 	echo "The file '$since_file' doesn't exist."
 	exit 1
@@ -58,7 +65,7 @@ if [ ! $? -eq 0 ] ; then
 	exit 1
 fi
 
-URL="${URL}/${resource_type}/?"
+URL="${URL}/${resource_type}/?project=${project}&"
 
 callcmd () # <patchwork id>
 {
-- 
2.25.1

Re: [dpdk-ci] [PATCH v2 09/10] tools: filter new patchwork IDs by project name

[Thomas Monjalon]

21/09/2021 16:35, alialnu@nvidia.com:
> From: Ali Alnubani <alialnu@nvidia.com>
> 
> Different projects require different checks usually, this
> patch modifies the script so that it requires a project's name
> which it passes as a parameter with the API end point.

I'm not satisfied with this explanation.
Please explain what happens if the project is not specified,
why this change is done.
My explanation: IDs are common for all projects.
If we want to check patches of a specific project, we need this filter.

[dpdk-ci] [PATCH v2 10/10] tools: skip the IDs we already fetched

[alialnu]

From: Ali Alnubani <alialnu@nvidia.com>

Store the IDs we already fetched in a file and don't
run 'callcmd' again for them.

Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
---
 tools/poll-pw | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/tools/poll-pw b/tools/poll-pw
index e197a91..dacf34b 100755
--- a/tools/poll-pw
+++ b/tools/poll-pw
@@ -65,6 +65,11 @@ if [ ! $? -eq 0 ] ; then
 	exit 1
 fi
 
+poll_pw_ids_file=/tmp/poll_pw_${resource_type}_ids
+if [ ! -f "$poll_pw_ids_file" ] ; then
+	touch $poll_pw_ids_file
+fi
+
 URL="${URL}/${resource_type}/?project=${project}&"
 
 callcmd () # <patchwork id>
@@ -77,7 +82,11 @@ while true ; do
 	date_now=$(date '+%FT%T')
 	since=$(date -d $(cat $since_file | tr '\n' ' ') '+%FT%T')
 	for id in $(curl -s "${URL}since=${since}" | jq '.[].id') ; do
+		if grep -q "^${id}$" $poll_pw_ids_file ; then
+			continue
+		fi
 		callcmd $id
+		echo $id >>$poll_pw_ids_file
 	done
 	echo -n $date_now >$since_file
 	# pause before next check
-- 
2.25.1

Re: [dpdk-ci] [PATCH v2 10/10] tools: skip the IDs we already fetched

[Thomas Monjalon]

21/09/2021 16:35, alialnu@nvidia.com:
> From: Ali Alnubani <alialnu@nvidia.com>
> 
> Store the IDs we already fetched in a file and don't
> run 'callcmd' again for them.

We store all IDs. Should we manually remove olds one from time to time?

We need an explanation about the strategy, why it is needed.
I think it is because filtering by date is not enough.
In order to not miss any patch, we should request a date earlier
than the previous fetch and skip those already fetched.

Where the "earlier date" is defined?

> Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
> ---
>  tools/poll-pw | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/tools/poll-pw b/tools/poll-pw
> index e197a91..dacf34b 100755
> --- a/tools/poll-pw
> +++ b/tools/poll-pw
> @@ -65,6 +65,11 @@ if [ ! $? -eq 0 ] ; then
>  	exit 1
>  fi
>  
> +poll_pw_ids_file=/tmp/poll_pw_${resource_type}_ids
> +if [ ! -f "$poll_pw_ids_file" ] ; then
> +	touch $poll_pw_ids_file
> +fi
> +
>  URL="${URL}/${resource_type}/?project=${project}&"
>  
>  callcmd () # <patchwork id>
> @@ -77,7 +82,11 @@ while true ; do
>  	date_now=$(date '+%FT%T')
>  	since=$(date -d $(cat $since_file | tr '\n' ' ') '+%FT%T')
>  	for id in $(curl -s "${URL}since=${since}" | jq '.[].id') ; do
> +		if grep -q "^${id}$" $poll_pw_ids_file ; then
> +			continue
> +		fi
>  		callcmd $id
> +		echo $id >>$poll_pw_ids_file
>  	done
>  	echo -n $date_now >$since_file
>  	# pause before next check
> 

Re: [dpdk-ci] [PATCH v2 10/10] tools: skip the IDs we already fetched

[Ali Alnubani]

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Thursday, September 30, 2021 1:33 PM
> To: Ali Alnubani <alialnu@nvidia.com>
> Cc: ci@dpdk.org; jerinj@marvell.com; ferruh.yigit@intel.com;
> david.marchand@redhat.com; juraj.linkes@pantheon.tech
> Subject: Re: [PATCH v2 10/10] tools: skip the IDs we already fetched
> 
> 21/09/2021 16:35, alialnu@nvidia.com:
> > From: Ali Alnubani <alialnu@nvidia.com>
> >
> > Store the IDs we already fetched in a file and don't
> > run 'callcmd' again for them.
> 
> We store all IDs. Should we manually remove olds one from time to time?
> 

Do you have a suggestion for when should we clear this file? Maybe each time the script starts?

> We need an explanation about the strategy, why it is needed.
> I think it is because filtering by date is not enough.
> In order to not miss any patch, we should request a date earlier
> than the previous fetch and skip those already fetched.
> 

The reason this change was made isn't because filtering by date is not enough, it's because
I want to avoid feeding the same ID to 'callcmd' more than once.
This can happen if a patchwork ID was created between recording date_now and fetching the API.
I don't think we are missing any IDs, even without this change.

> Where the "earlier date" is defined?

There are 2 variables, "date_now", which is recorded right before fetching from the API, and then gets written
to the file, and "since", which is the last date that was written to the file.

> 
> > Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
> > ---
> >  tools/poll-pw | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/tools/poll-pw b/tools/poll-pw
> > index e197a91..dacf34b 100755
> > --- a/tools/poll-pw
> > +++ b/tools/poll-pw
> > @@ -65,6 +65,11 @@ if [ ! $? -eq 0 ] ; then
> >  	exit 1
> >  fi
> >
> > +poll_pw_ids_file=/tmp/poll_pw_${resource_type}_ids
> > +if [ ! -f "$poll_pw_ids_file" ] ; then
> > +	touch $poll_pw_ids_file
> > +fi
> > +
> >  URL="${URL}/${resource_type}/?project=${project}&"
> >
> >  callcmd () # <patchwork id>
> > @@ -77,7 +82,11 @@ while true ; do
> >  	date_now=$(date '+%FT%T')
> >  	since=$(date -d $(cat $since_file | tr '\n' ' ') '+%FT%T')
> >  	for id in $(curl -s "${URL}since=${since}" | jq '.[].id') ; do
> > +		if grep -q "^${id}$" $poll_pw_ids_file ; then
> > +			continue
> > +		fi
> >  		callcmd $id
> > +		echo $id >>$poll_pw_ids_file
> >  	done
> >  	echo -n $date_now >$since_file
> >  	# pause before next check
> >
> 
> 
> 
> 

Re: [dpdk-ci] [PATCH v2 10/10] tools: skip the IDs we already fetched

[Thomas Monjalon]

11/10/2021 21:30, Ali Alnubani:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 21/09/2021 16:35, alialnu@nvidia.com:
> > > From: Ali Alnubani <alialnu@nvidia.com>
> > >
> > > Store the IDs we already fetched in a file and don't
> > > run 'callcmd' again for them.
> > 
> > We store all IDs. Should we manually remove olds one from time to time?
> > 
> 
> Do you have a suggestion for when should we clear this file? Maybe each time the script starts?

Yes at each start, we can remove the very old entries, like more than 10 hours old.

> > We need an explanation about the strategy, why it is needed.
> > I think it is because filtering by date is not enough.
> > In order to not miss any patch, we should request a date earlier
> > than the previous fetch and skip those already fetched.
> 
> The reason this change was made isn't because filtering by date is not enough, it's because
> I want to avoid feeding the same ID to 'callcmd' more than once.
> This can happen if a patchwork ID was created between recording date_now and fetching the API.
> I don't think we are missing any IDs, even without this change.
> 
> > Where the "earlier date" is defined?
> 
> There are 2 variables, "date_now", which is recorded right before fetching from the API, and then gets written
> to the file, and "since", which is the last date that was written to the file.

OK please update the commit log.

Re: [dpdk-ci] [PATCH v2 10/10] tools: skip the IDs we already fetched

[Ali Alnubani]

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Tuesday, October 12, 2021 9:45 AM
> To: Ali Alnubani <alialnu@nvidia.com>
> Cc: ci@dpdk.org; jerinj@marvell.com; ferruh.yigit@intel.com;
> david.marchand@redhat.com; juraj.linkes@pantheon.tech
> Subject: Re: [PATCH v2 10/10] tools: skip the IDs we already fetched
> 
> 11/10/2021 21:30, Ali Alnubani:
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 21/09/2021 16:35, alialnu@nvidia.com:
> > > > From: Ali Alnubani <alialnu@nvidia.com>
> > > >
> > > > Store the IDs we already fetched in a file and don't
> > > > run 'callcmd' again for them.
> > >
> > > We store all IDs. Should we manually remove olds one from time to time?
> > >
> >
> > Do you have a suggestion for when should we clear this file? Maybe each
> time the script starts?
> 
> Yes at each start, we can remove the very old entries, like more than 10
> hours old.

Wouldn't this require using and depending on a database (at least an SQLite one) to additionally store timestamps to use for queries?
Do you have a simpler way in mind to keep track of the IDs and their timestamps to process later based on how long they have been in the file? Maybe keep only up to a certain number of IDs in this file (up to a 1000 maybe)?

> 
> > > We need an explanation about the strategy, why it is needed.
> > > I think it is because filtering by date is not enough.
> > > In order to not miss any patch, we should request a date earlier
> > > than the previous fetch and skip those already fetched.
> >
> > The reason this change was made isn't because filtering by date is not
> enough, it's because
> > I want to avoid feeding the same ID to 'callcmd' more than once.
> > This can happen if a patchwork ID was created between recording
> date_now and fetching the API.
> > I don't think we are missing any IDs, even without this change.
> >
> > > Where the "earlier date" is defined?
> >
> > There are 2 variables, "date_now", which is recorded right before fetching
> from the API, and then gets written
> > to the file, and "since", which is the last date that was written to the file.
> 
> OK please update the commit log.
> 

Updated commit messages in v3.

Re: [dpdk-ci] [PATCH v2 10/10] tools: skip the IDs we already fetched

[Thomas Monjalon]

18/10/2021 10:04, Ali Alnubani:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 11/10/2021 21:30, Ali Alnubani:
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > 21/09/2021 16:35, alialnu@nvidia.com:
> > > > > From: Ali Alnubani <alialnu@nvidia.com>
> > > > >
> > > > > Store the IDs we already fetched in a file and don't
> > > > > run 'callcmd' again for them.
> > > >
> > > > We store all IDs. Should we manually remove olds one from time to time?
> > >
> > > Do you have a suggestion for when should we clear this file? Maybe each
> > time the script starts?
> > 
> > Yes at each start, we can remove the very old entries, like more than 10
> > hours old.
> 
> Wouldn't this require using and depending on a database (at least an SQLite one) to additionally store timestamps to use for queries?
> Do you have a simpler way in mind to keep track of the IDs and their timestamps to process later based on how long they have been in the file? Maybe keep only up to a certain number of IDs in this file (up to a 1000 maybe)?

Yes we can store each entry as a line starting with timestamp:
	timestamp ID

Re: [dpdk-ci] [PATCH v2 10/10] tools: skip the IDs we already fetched

[Ali Alnubani]

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Tuesday, October 26, 2021 5:07 PM
> To: Ali Alnubani <alialnu@nvidia.com>
> Cc: ci@dpdk.org; jerinj@marvell.com; ferruh.yigit@intel.com;
> david.marchand@redhat.com; juraj.linkes@pantheon.tech
> Subject: Re: [PATCH v2 10/10] tools: skip the IDs we already fetched
> 
> 18/10/2021 10:04, Ali Alnubani:
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 11/10/2021 21:30, Ali Alnubani:
> > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > 21/09/2021 16:35, alialnu@nvidia.com:
> > > > > > From: Ali Alnubani <alialnu@nvidia.com>
> > > > > >
> > > > > > Store the IDs we already fetched in a file and don't
> > > > > > run 'callcmd' again for them.
> > > > >
> > > > > We store all IDs. Should we manually remove olds one from time to
> time?
> > > >
> > > > Do you have a suggestion for when should we clear this file? Maybe
> each
> > > time the script starts?
> > >
> > > Yes at each start, we can remove the very old entries, like more than 10
> > > hours old.
> >
> > Wouldn't this require using and depending on a database (at least an SQLite
> one) to additionally store timestamps to use for queries?
> > Do you have a simpler way in mind to keep track of the IDs and their
> timestamps to process later based on how long they have been in the file?
> Maybe keep only up to a certain number of IDs in this file (up to a 1000
> maybe)?
> 
> Yes we can store each entry as a line starting with timestamp:
> 	timestamp ID

If the purpose is to not allow the script to grow indefinitely, is it ok to just cut the file by half once it reaches a specific number of lines (like 1000)?.
Do you agree with this approach? It'd be easier to implement compared to storing and processing timestamps.

Re: [dpdk-ci] [PATCH v2 10/10] tools: skip the IDs we already fetched

[Thomas Monjalon]

04/11/2021 17:53, Ali Alnubani:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 18/10/2021 10:04, Ali Alnubani:
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > 11/10/2021 21:30, Ali Alnubani:
> > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > 21/09/2021 16:35, alialnu@nvidia.com:
> > > > > > > From: Ali Alnubani <alialnu@nvidia.com>
> > > > > > >
> > > > > > > Store the IDs we already fetched in a file and don't
> > > > > > > run 'callcmd' again for them.
> > > > > >
> > > > > > We store all IDs. Should we manually remove olds one from time to
> > time?
> > > > >
> > > > > Do you have a suggestion for when should we clear this file? Maybe
> > each
> > > > time the script starts?
> > > >
> > > > Yes at each start, we can remove the very old entries, like more than 10
> > > > hours old.
> > >
> > > Wouldn't this require using and depending on a database (at least an SQLite
> > one) to additionally store timestamps to use for queries?
> > > Do you have a simpler way in mind to keep track of the IDs and their
> > timestamps to process later based on how long they have been in the file?
> > Maybe keep only up to a certain number of IDs in this file (up to a 1000
> > maybe)?
> > 
> > Yes we can store each entry as a line starting with timestamp:
> > 	timestamp ID
> 
> If the purpose is to not allow the script to grow indefinitely, is it ok to just cut the file by half once it reaches a specific number of lines (like 1000)?.
> Do you agree with this approach? It'd be easier to implement compared to storing and processing timestamps.

Yes it is simpler and should work.

Re: [dpdk-ci] [PATCH v2 10/10] tools: skip the IDs we already fetched

[Ali Alnubani]

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Thursday, November 4, 2021 8:09 PM
> To: Ali Alnubani <alialnu@nvidia.com>
> Cc: ci@dpdk.org; jerinj@marvell.com; ferruh.yigit@intel.com;
> david.marchand@redhat.com; juraj.linkes@pantheon.tech
> Subject: Re: [PATCH v2 10/10] tools: skip the IDs we already fetched
> 
> 04/11/2021 17:53, Ali Alnubani:
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 18/10/2021 10:04, Ali Alnubani:
> > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > 11/10/2021 21:30, Ali Alnubani:
> > > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > > 21/09/2021 16:35, alialnu@nvidia.com:
> > > > > > > > From: Ali Alnubani <alialnu@nvidia.com>
> > > > > > > >
> > > > > > > > Store the IDs we already fetched in a file and don't
> > > > > > > > run 'callcmd' again for them.
> > > > > > >
> > > > > > > We store all IDs. Should we manually remove olds one from time
> to
> > > time?
> > > > > >
> > > > > > Do you have a suggestion for when should we clear this file? Maybe
> > > each
> > > > > time the script starts?
> > > > >
> > > > > Yes at each start, we can remove the very old entries, like more than
> 10
> > > > > hours old.
> > > >
> > > > Wouldn't this require using and depending on a database (at least an
> SQLite
> > > one) to additionally store timestamps to use for queries?
> > > > Do you have a simpler way in mind to keep track of the IDs and their
> > > timestamps to process later based on how long they have been in the
> file?
> > > Maybe keep only up to a certain number of IDs in this file (up to a 1000
> > > maybe)?
> > >
> > > Yes we can store each entry as a line starting with timestamp:
> > > 	timestamp ID
> >
> > If the purpose is to not allow the script to grow indefinitely, is it ok to just
> cut the file by half once it reaches a specific number of lines (like 1000)?.
> > Do you agree with this approach? It'd be easier to implement compared to
> storing and processing timestamps.
> 
> Yes it is simpler and should work.

The file is now cut in half each time it reaches 1000 entries. See v4:
https://inbox.dpdk.org/ci/20211108062815.7614-11-alialnu@nvidia.com/T/#u

[dpdk-ci] [PATCH v4 00/10] Automatic patchwork delegation

[Ali Alnubani]

This patchset adds support for automatic patch delegation.

The script 'guess_git_tree.py' was renamed to 'pw_maintainers_cli.py'
and now supports finding patch and series maintainers. It can also
delegate patches to them using git-pw.

The script 'poll-pw' was rewritten to fetch new api resources
by filtering with date and project. It now supports fetching both
patches and series.

Ali Alnubani (10):
  tools: rename guess_git_tree script
  tools: match by tree URL instead of tree name
  tools: update script usage
  tools: add functionality for detecting tree maintainers
  tools: add functionality for setting pw delegates
  add git-pw to requirements file
  tools: filter new Patchwork IDs by date
  tools: support fetching series
  tools: filter new patchwork IDs by project name
  tools: skip the IDs we already fetched

 requirements.txt                              |   3 +-
 tools/poll-pw                                 |  97 ++++++++++---
 ...uess_git_tree.py => pw_maintainers_cli.py} | 137 ++++++++++++++----
 3 files changed, 187 insertions(+), 50 deletions(-)
 rename tools/{guess_git_tree.py => pw_maintainers_cli.py} (64%)

-- 
2.25.1

[dpdk-ci] [PATCH v4 01/10] tools: rename guess_git_tree script

[Ali Alnubani]

The name 'pw_maintainers_cli.py' will make more sense when
adding more operations to the script.

Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
---
Changes in v2:
- Changed the script's name to 'pw_maintainers_cli.py' instead of
  'maintainers.py' (Suggested by Juraj Linkes).

 tools/{guess_git_tree.py => pw_maintainers_cli.py} | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
 rename tools/{guess_git_tree.py => pw_maintainers_cli.py} (98%)

diff --git a/tools/guess_git_tree.py b/tools/pw_maintainers_cli.py
similarity index 98%
rename from tools/guess_git_tree.py
rename to tools/pw_maintainers_cli.py
index c9eef39..a31f605 100755
--- a/tools/guess_git_tree.py
+++ b/tools/pw_maintainers_cli.py
@@ -31,13 +31,13 @@ variables PW_{SERVER,PROJECT,TOKEN} should be set. If not, the script will try
 to load the git configurations pw.{server,project,token}.
 
 Example usage:
-    ./guess-git-tree.py --command list_trees_for_series 2054
-    ./guess-git-tree.py --command list_trees_for_patch 2054
+    ./pw_maintainers_cli.py --command list_trees_for_series 2054
+    ./pw_maintainers_cli.py --command list_trees_for_patch 2054
 
 Or if you want to use inside other scripts:
 
     import os
-    from guess_git_tree import (Maintainers, GitPW, Diff)
+    from pw_maintainers_cli import (Maintainers, GitPW, Diff)
     _git_pw = GitPW({
         'pw_server': os.environ.get('PW_SERVER'),
         'pw_project': os.environ.get('PW_PROJECT'),
-- 
2.25.1

[dpdk-ci] [PATCH v4 02/10] tools: match by tree URL instead of tree name

[Ali Alnubani]

The tree URL has more information. It can be used to
get the maintainer of a tree, and it includes the tree name anyway.
Although we can construct the tree URL by prepending
"git://dpdk.org/next/" to the trees that have "next-" in their name,
and prepending "git://dpdk.org/" to anything else, it would be safer
to read the URL from the MAINTAINERS file just in case new trees that
don't follow this rule were added.

Also try to use named capture groups as much as possible.

Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
---
 tools/pw_maintainers_cli.py | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/tools/pw_maintainers_cli.py b/tools/pw_maintainers_cli.py
index a31f605..343e9f5 100755
--- a/tools/pw_maintainers_cli.py
+++ b/tools/pw_maintainers_cli.py
@@ -46,7 +46,8 @@ Or if you want to use inside other scripts:
     maintainers = Maintainers()
     patch_id = 52199
     files = Diff.find_filenames(_git_pw.api_get('patches', patch_id)['diff'])
-    tree = maintainers.get_tree(files)
+    tree_url = maintainers.get_tree(files)
+    tree_name = tree_url.split('/')[-1]
 """
 
 
@@ -116,9 +117,9 @@ class Diff(object):
 class Maintainers(object):
 
     file_regex = r'F:\s(.*)'
-    tree_regex = r'T: git:\/\/dpdk\.org(?:\/next)*\/(.*)'
+    tree_regex = r'T: (?P<url>git:\/\/dpdk\.org(?:\/next)*\/(?P<name>.*))'
     section_regex = r'([^\n]*)\n-+.*?(?=([^\n]*\n-+)|\Z)'
-    subsection_regex = r'[^\n](?:(?!\n{{2}}).)*?^F: {}(?:(?!\n{{2}}).)*'
+    subsection_regex = r'[^\n](?:(?!\n{{2}}).)*?^{}: {}$(?:(?!\n{{2}}).)*'
 
     def __init__(self):
         with open(MAINTAINERS_FILE_PATH) as fd:
@@ -151,8 +152,8 @@ class Maintainers(object):
             if _tree:
                 tree_list.append(_tree)
         tree = self.get_common_denominator(tree_list)
-        if tree == '':
-            tree = 'dpdk'
+        if not tree:
+            tree = 'git://dpdk.org/dpdk'
         return tree
 
     def _get_tree(self, filename):
@@ -180,7 +181,7 @@ class Maintainers(object):
 
         found_match = False
         # Find the block containing filename.
-        regex = self.subsection_regex.format(re.escape(matching_pattern))
+        regex = self.subsection_regex.format('F', re.escape(matching_pattern))
         subsection_match = re.findall(
                 regex,
                 self.maintainers_txt,
@@ -191,7 +192,7 @@ class Maintainers(object):
             tree_match = re.search(
                     self.tree_regex, subsection)
             if tree_match:
-                tree = tree_match.group(1)
+                tree = tree_match.group('url')
                 self.matched[matching_pattern] = tree
                 found_match = True
 
@@ -204,7 +205,7 @@ class Maintainers(object):
                             self.tree_regex,
                             section.group(0).split('\n\n')[0])
                     if tree_match:
-                        tree = tree_match.group(1)
+                        tree = tree_match.group('url')
 
         self.matched[matching_pattern] = tree
         return tree
@@ -228,8 +229,8 @@ class Maintainers(object):
             os.path.commonprefix(_tree_list).rstrip('-').replace(
                     'dpdk-next-net-virtio', 'dpdk-next-virtio')
         # There is no 'dpdk-next' named tree.
-        if common_prefix == 'dpdk-next':
-            common_prefix = 'dpdk'
+        if common_prefix.endswith('dpdk-next') or common_prefix.endswith('/'):
+            common_prefix = 'git://dpdk.org/dpdk'
         return common_prefix
 
 
@@ -289,4 +290,4 @@ if __name__ == '__main__':
     files = []
     for patch in patch_list:
         files += Diff.find_filenames(patch['diff'])
-    print(maintainers.get_tree(files))
+    print(maintainers.get_tree(files).split('/')[-1])
-- 
2.25.1

[dpdk-ci] [PATCH v4 03/10] tools: update script usage

[Ali Alnubani]

The resource type (whether it's a patch or a series) is now
specified by the new argument --type.
Both commands: list_trees_for_patch & list_trees_for_series
are replaced by the positional arg 'list-trees', and the type must
be always set. This makes adding more operations require less arguments.

Additionally, use the more common dashes in argument names instead
of underscores.

Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
---
Changes in v4:
- Use dashes in argument names instead of underscores.

 tools/pw_maintainers_cli.py | 38 +++++++++++++++++++++++--------------
 1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/tools/pw_maintainers_cli.py b/tools/pw_maintainers_cli.py
index 343e9f5..67d8586 100755
--- a/tools/pw_maintainers_cli.py
+++ b/tools/pw_maintainers_cli.py
@@ -31,8 +31,8 @@ variables PW_{SERVER,PROJECT,TOKEN} should be set. If not, the script will try
 to load the git configurations pw.{server,project,token}.
 
 Example usage:
-    ./pw_maintainers_cli.py --command list_trees_for_series 2054
-    ./pw_maintainers_cli.py --command list_trees_for_patch 2054
+    ./pw_maintainers_cli.py --type series list-trees 2054
+    ./pw_maintainers_cli.py --type patch list-trees 2054
 
 Or if you want to use inside other scripts:
 
@@ -238,36 +238,42 @@ if __name__ == '__main__':
     """Main procedure."""
     parser = argparse.ArgumentParser()
     git_pw_conf_parser = parser.add_argument_group('git-pw configurations')
-    options_parser = parser.add_argument_group('optional arguments')
+    required_args_parser = parser.add_argument_group('required arguments')
 
-    options_parser.add_argument(
-            '--command',
+    required_args_parser.add_argument(
+            '--type',
             choices=(
-                'list_trees_for_patch',
-                'list_trees_for_series'),
-            required=True, help='Command to perform')
+                'patch',
+                'series'),
+            required=True, help='Resource type.')
 
     git_pw_conf_parser.add_argument(
-            '--pw_server', type=str,
+            '--pw-server', type=str,
             default=os.environ.get(
                 'PW_SERVER', utils.git_config('pw.server')),
             help='Patchwork server')
     git_pw_conf_parser.add_argument(
-            '--pw_project', type=str,
+            '--pw-project', type=str,
             default=os.environ.get(
                 'PW_PROJECT', utils.git_config('pw.project')),
             help='Patchwork project')
     git_pw_conf_parser.add_argument(
-            '--pw_token', type=str,
+            '--pw-token', type=str,
             default=os.environ.get('PW_TOKEN', utils.git_config('pw.token')),
             help='Authentication token')
 
+    parser.add_argument(
+            'command',
+            choices=[
+                'list-trees'],
+            help='Command to perform')
     parser.add_argument(
             'id', type=int, help='patch/series id')
 
     args = parser.parse_args()
 
     command = args.command
+    resource_type = args.type
     _id = args.id
 
     # Pass the needed configurations to git-pw.
@@ -279,9 +285,9 @@ if __name__ == '__main__':
     maintainers = Maintainers()
 
     patch_list = []
-    if command == 'list_trees_for_patch':
+    if resource_type == 'patch':
         patch_list.append(_git_pw.api_get('patches', _id))
-    elif command == 'list_trees_for_series':
+    else:
         series = _git_pw.api_get('series', _id)
         patch_list = [
                 _git_pw.api_get('patches', patch['id'])
@@ -290,4 +296,8 @@ if __name__ == '__main__':
     files = []
     for patch in patch_list:
         files += Diff.find_filenames(patch['diff'])
-    print(maintainers.get_tree(files).split('/')[-1])
+
+    tree = maintainers.get_tree(files)
+
+    if command == 'list-trees':
+        print(tree.split('/')[-1])
-- 
2.25.1

[dpdk-ci] [PATCH v4 04/10] tools: add functionality for detecting tree maintainers

[Ali Alnubani]

Detecting a maintainer works by searching the
'General Project Administration' section for subsections
containing the provided tree, and then returning the maintainers
specified in that subsection.

Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
---
Changes in v3:
- Stored 'General Project Administration' in a variable (Suggested by
  Thomas Monjalon).

 tools/pw_maintainers_cli.py | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/tools/pw_maintainers_cli.py b/tools/pw_maintainers_cli.py
index 67d8586..904be2e 100755
--- a/tools/pw_maintainers_cli.py
+++ b/tools/pw_maintainers_cli.py
@@ -33,6 +33,7 @@ to load the git configurations pw.{server,project,token}.
 Example usage:
     ./pw_maintainers_cli.py --type series list-trees 2054
     ./pw_maintainers_cli.py --type patch list-trees 2054
+    ./pw_maintainers_cli.py --type patch list-maintainers 2054
 
 Or if you want to use inside other scripts:
 
@@ -48,6 +49,7 @@ Or if you want to use inside other scripts:
     files = Diff.find_filenames(_git_pw.api_get('patches', patch_id)['diff'])
     tree_url = maintainers.get_tree(files)
     tree_name = tree_url.split('/')[-1]
+    maintainers = maintainers.get_maintainers(tree_url)
 """
 
 
@@ -118,8 +120,10 @@ class Maintainers(object):
 
     file_regex = r'F:\s(.*)'
     tree_regex = r'T: (?P<url>git:\/\/dpdk\.org(?:\/next)*\/(?P<name>.*))'
+    maintainer_regex = r'M:\s(.*)'
     section_regex = r'([^\n]*)\n-+.*?(?=([^\n]*\n-+)|\Z)'
     subsection_regex = r'[^\n](?:(?!\n{{2}}).)*?^{}: {}$(?:(?!\n{{2}}).)*'
+    general_proj_admin_title = 'General Project Administration'
 
     def __init__(self):
         with open(MAINTAINERS_FILE_PATH) as fd:
@@ -141,6 +145,26 @@ class Maintainers(object):
         # Save already matched patterns.
         self.matched = {}
 
+    def get_maintainers(self, tree):
+        """
+        Return a list of a tree's maintainers."""
+        maintainers = []
+        for section in self.sections:
+            if section.group(1) == self.general_proj_admin_title:
+                # Find the block containing the tree.
+                regex = self.subsection_regex.format('T', re.escape(tree))
+                subsection_match = re.findall(
+                        regex,
+                        section.group(0),
+                        re.DOTALL | re.MULTILINE)
+                if len(subsection_match):
+                    subsection = subsection_match[-1]
+                    # Look for maintainers
+                    maintainers = re.findall(
+                            self.maintainer_regex, subsection)
+                    return maintainers
+                break
+
     def get_tree(self, files):
         """
         Return a git tree that matches a list of files."""
@@ -265,7 +289,7 @@ if __name__ == '__main__':
     parser.add_argument(
             'command',
             choices=[
-                'list-trees'],
+                'list-trees', 'list-maintainers'],
             help='Command to perform')
     parser.add_argument(
             'id', type=int, help='patch/series id')
@@ -301,3 +325,5 @@ if __name__ == '__main__':
 
     if command == 'list-trees':
         print(tree.split('/')[-1])
+    elif command == 'list-maintainers':
+        print(*maintainers.get_maintainers(tree), sep='\n')
-- 
2.25.1

[dpdk-ci] [PATCH v4 05/10] tools: add functionality for setting pw delegates

[Ali Alnubani]

A new command was added to set patch delegates in Patchwork
based on the emails found in DPDK's MAINTAINERS file.

Example usage:
  $ export MAINTAINERS_FILE_PATH=/path/to/dpdk/MAINTAINERS
  $ ./pw_maintainers_cli.py --type series set-pw-delegate SERIES_ID

Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
---
Changes in v3:
- Added an argument to force overriding delegates.
Changes in v4:
- Force overriding delegates by default, and add an arg to
  change that behavior (Suggested by Thomas Monjalon).
- Shorten link in set_delegate's doc.

 tools/pw_maintainers_cli.py | 56 +++++++++++++++++++++++++++++++++++--
 1 file changed, 53 insertions(+), 3 deletions(-)

diff --git a/tools/pw_maintainers_cli.py b/tools/pw_maintainers_cli.py
index 904be2e..4527cf7 100755
--- a/tools/pw_maintainers_cli.py
+++ b/tools/pw_maintainers_cli.py
@@ -14,6 +14,7 @@ from requests.exceptions import HTTPError
 from git_pw import config
 from git_pw import api
 from git_pw import utils
+from git_pw import patch as git_pw_patch
 
 """
 Description:
@@ -84,6 +85,33 @@ class GitPW(object):
             else:
                 raise
 
+    def set_delegate(self, patch_list, delegate, skip_delegated=False):
+        """Set the delegate for a patch.
+        This overrides the current delegate. If 'skip_delegated' is set to
+        True, only set a delegate for patches that don't have one set already.
+
+        Reference:
+        https://github.com/getpatchwork/git-pw/blob/76b79097dc0a57/git_pw/patch.py#L167
+        """
+        users = api.index('users', [('q', delegate)])
+        if len(users) != 1:
+            # Zero or multiple users found
+            print('Cannot choose a Patchwork user to delegate to from '
+                  'user list ({}). Skipping..'.format(users))
+            return
+        for patch in patch_list:
+            if patch['delegate'] != None and \
+                    (patch['delegate'].get('email') == users[0].get('email') or \
+                    skip_delegated):
+                print('Patch {} is already delegated to {}. '
+                      'Skipping..'.format(
+                          patch['id'], patch['delegate']['email']))
+                continue
+            print("Delegating patch {} to {}..".format(
+                patch['id'], users[0]['email']))
+            _ = api.update(
+                    'patches', patch['id'], [('delegate', users[0]['id'])])
+
 
 class Diff(object):
 
@@ -286,16 +314,21 @@ if __name__ == '__main__':
             default=os.environ.get('PW_TOKEN', utils.git_config('pw.token')),
             help='Authentication token')
 
+    parser.add_argument(
+            '--skip-delegated',
+            action='store_true', required=False,
+            help='Skip patches that are already delegated')
     parser.add_argument(
             'command',
             choices=[
-                'list-trees', 'list-maintainers'],
+                'list-trees', 'list-maintainers', 'set-pw-delegate'],
             help='Command to perform')
     parser.add_argument(
             'id', type=int, help='patch/series id')
 
     args = parser.parse_args()
 
+    skip_delegated = args.skip_delegated
     command = args.command
     resource_type = args.type
     _id = args.id
@@ -325,5 +358,22 @@ if __name__ == '__main__':
 
     if command == 'list-trees':
         print(tree.split('/')[-1])
-    elif command == 'list-maintainers':
-        print(*maintainers.get_maintainers(tree), sep='\n')
+    if command in ['list-maintainers', 'set-pw-delegate']:
+        maintainer_list = maintainers.get_maintainers(tree)
+        if command == 'list-maintainers':
+            print(*maintainer_list, sep='\n')
+        elif command == 'set-pw-delegate':
+            if len(maintainer_list) > 0:
+                # Get the email of the first maintainer in the list.
+                try:
+                    delegate = re.match(
+                            r".*\<(?P<email>.*)\>",
+                            maintainer_list[0]).group('email')
+                except AttributeError:
+                    print("Unexpected format: '{}'".format(maintainer_list[0]))
+                    sys.exit(1)
+                _git_pw.set_delegate(
+                        patch_list, delegate,
+                        skip_delegated=skip_delegated)
+            else:
+                print('No maintainers found. Not setting a delegate.')
-- 
2.25.1

[dpdk-ci] [PATCH v4 06/10] add git-pw to requirements file

[Ali Alnubani]

The module has always been required by tools/maintainers.py (previously
named tools/guess_git_tree.py).

Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
---
 requirements.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/requirements.txt b/requirements.txt
index f20067d..f2a6844 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1 +1,2 @@
-whatthepatch==1.0.2
\ No newline at end of file
+git-pw==2.1.0
+whatthepatch==1.0.2
-- 
2.25.1

[dpdk-ci] [PATCH v4 07/10] tools: filter new Patchwork IDs by date

[Ali Alnubani]

API resource IDs are guaranteed to be unique, but they aren't guaranteed
to have no gaps, for example, the following series IDs are
nonexistent: 16157, 17181, 18235.
Filtering by the date since the last check is necessary to later
add support for fetching new series IDs in addition to patch IDs.

The script now periodically fetches Patchwork's events API filtering
by the 'patch-completed' category (see [1]). It parses the responses using jq
and passes the IDs to 'callcmd'.

Instead of requiring a file that contains the next patch ID,
a file containing the timestamp of the last time the API was fetched
is now used. Each time the API is fetched for new patches, the timestamp
in the file gets updated, and the script sleeps an amount of time specified
by PAUSE_SECONDS before attempting to fetch new resources again.
There are 2 variables, 'date_now', which is recorded right before
fetching from the API, and then gets written to the file, and 'since',
which is the last date that was written to the file.

Since API responses can be in multiple pages, the script will keep
attempting to fetch the next page until jq encounters an error while
parsing a non list response with no 'payload' key.

The pause amount between each poll attempt is still 100 seconds.

The script writes the date in Universal Time (UTC) format and
expects to read the same format from the file as Patchwork stores event
objects with a naive datetime object that is not aware of the
server's timezone (see [2] and [3]).

The package jq (Command-line JSON processor) is now required by the
script.

Example usage:
$ export MAINTAINERS_FILE_PATH=/path/to/dpdk/MAINTAINERS
$ ./tools/poll-pw /path/to/last.txt \
    '/path/to/pw_maintainers_cli.py --type patch set-pw-delegate $1'

[1] https://patchwork.readthedocs.io/en/latest/usage/overview/#patch-completed
[2] https://github.com/getpatchwork/patchwork/blob/580cc8570a05c1/patchwork/models.py#L1058
[3] https://docs.python.org/3/library/datetime.html#datetime.datetime.utcnow

Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
---
Changes in v2:
- Removed an unnecessary 'break' statement in the for loop.
Changes in v3:
- Fetch the events API endpoint for new patches as the date field in the
  /patches and /series endpoints is the mbox creation date, not
  the date they were created in Patchwork.
- Use UTC date format.
- Redirect error messages to stderr.
- Fixed typo in usage (specifed -> specified).

 tools/poll-pw | 63 ++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 42 insertions(+), 21 deletions(-)

diff --git a/tools/poll-pw b/tools/poll-pw
index bdf860a..ccc58f0 100755
--- a/tools/poll-pw
+++ b/tools/poll-pw
@@ -1,58 +1,79 @@
-#! /bin/sh -e
+#! /bin/sh
 
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright 2017 6WIND S.A.
 # Copyright 2018 Mellanox Technologies, Ltd
 
 URL=http://patches.dpdk.org/api
+PAUSE_SECONDS=100
 
 print_usage () {
 	cat <<- END_OF_HELP
-	usage: $(basename $0) <counter> <command>
+	usage: $(basename $0) [OPTIONS] </path/to/last.txt> <command>
 
 	Poll patchwork and call command for each patch.
-	The first patchwork id to be checked is read from the counter file.
-	The command should use '$1' to be evaluated as patchwork id.
-	When a patch is found and the command is successful,
-	then the counter is incremented.
+	The first date to filter with is read from the specified file.
+	The command should use '$1' to be evaluated as the patch id.
+	The date in the specified file is updated after each pull.
 	END_OF_HELP
 }
 
+which jq >/dev/null 2>&1
+if [ ! $? -eq 0 ] ; then
+	printf "The command 'jq' doesn't exist, please install it.\n\n" >&2
+	exit 1
+fi
+
 while getopts h arg ; do
 	case $arg in
 		h ) print_usage ; exit 0 ;;
 		? ) print_usage >&2 ; exit 1 ;;
 	esac
 done
+
 if [ $# -lt 2 ] ; then
-	printf 'missing argument\n\n' >&2
+	printf 'missing argument(s)\n\n' >&2
 	print_usage >&2
 	exit 1
 fi
 shift $(($OPTIND - 1))
-counter=$1
+since_file=$1
 shift
 cmd=$*
 
+if [ ! -f "$since_file" ] ; then
+	printf "The file '$since_file' doesn't exist.\n\n" >&2
+	exit 1
+fi
+
+date -d "$(cat $since_file | tr '\n' ' ')" >/dev/null 2>&1
+if [ ! $? -eq 0 ] ; then
+	printf "The file '$since_file' doesn't contain a valid date format.\n\n" >&2
+	exit 1
+fi
+
+URL="${URL}/events/?category=patch-completed"
+
 callcmd () # <patchwork id>
 {
 	eval $cmd
 }
 
-checkid () # <patchwork id>
-{
-	curl -sfIo /dev/null $URL/patches/$1/ ||
-	curl -sfIo /dev/null $URL/covers/$1/
-}
-
-pwid=$(cat $counter)
+set -e
 while true ; do
-	# process all recent patches
-	while checkid $pwid ; do
-		callcmd $pwid || break
-		pwid=$(($pwid + 1))
-		echo $pwid >$counter
+	date_now=$(date --utc '+%FT%T')
+	since=$(date -d $(cat $since_file | tr '\n' ' ') '+%FT%T')
+	page=1
+	while true ; do
+		ids=$(curl -s "${URL}&page=${page}&since=${since}" \
+			| jq 'try ( .[].payload.patch.id )')
+		[ -z "$(echo $ids | tr -d '\n')" ] && break
+		for id in $ids ; do
+			callcmd $id
+		done
+		page=$((page+1))
 	done
+	echo -n $date_now >$since_file
 	# pause before next check
-	sleep 100
+	sleep $PAUSE_SECONDS
 done
-- 
2.25.1

[dpdk-ci] [PATCH v4 08/10] tools: support fetching series

[Ali Alnubani]

The script can now fetch new series IDs by filtering the events
API with category 'series-completed'. See:
https://patchwork.readthedocs.io/en/latest/usage/overview/#series-completed

Example usage:
$ export MAINTAINERS_FILE_PATH=/path/to/dpdk/MAINTAINERS
$ ./tools/poll-pw series /path/to/last.txt \
    '/path/to/pw_maintainers_cli.py --type series set-pw-delegate $1'

Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
---
Changes in v3:
- Fetch new series IDs by filtering the events API with
  category "series-completed".
- Updated usage (renamed 'patches' arg to 'patch') so that
  it feeds directly into the 'patch-completed' category.

 tools/poll-pw | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/tools/poll-pw b/tools/poll-pw
index ccc58f0..48c9fd0 100755
--- a/tools/poll-pw
+++ b/tools/poll-pw
@@ -9,11 +9,11 @@ PAUSE_SECONDS=100
 
 print_usage () {
 	cat <<- END_OF_HELP
-	usage: $(basename $0) [OPTIONS] </path/to/last.txt> <command>
+	usage: $(basename $0) [OPTIONS] <patch|series> </path/to/last.txt> <command>
 
-	Poll patchwork and call command for each patch.
+	Poll patchwork and call a command for each new patch/series id.
 	The first date to filter with is read from the specified file.
-	The command should use '$1' to be evaluated as the patch id.
+	The command should use '$1' to be evaluated as the patch/series id.
 	The date in the specified file is updated after each pull.
 	END_OF_HELP
 }
@@ -31,14 +31,15 @@ while getopts h arg ; do
 	esac
 done
 
-if [ $# -lt 2 ] ; then
+if [ $# -lt 3 ] ; then
 	printf 'missing argument(s)\n\n' >&2
 	print_usage >&2
 	exit 1
 fi
 shift $(($OPTIND - 1))
-since_file=$1
-shift
+resource_type=$1
+since_file=$2
+shift 2
 cmd=$*
 
 if [ ! -f "$since_file" ] ; then
@@ -52,7 +53,13 @@ if [ ! $? -eq 0 ] ; then
 	exit 1
 fi
 
-URL="${URL}/events/?category=patch-completed"
+if [ "$resource_type" != "patch" ] & [ "$resource_type" != "series" ] ; then
+	printf "Unknown resource type '$resource_type'.\n\n" >&2
+	print_usage >&2
+	exit 1
+fi
+
+URL="${URL}/events/?category=${resource_type}-completed"
 
 callcmd () # <patchwork id>
 {
@@ -66,7 +73,7 @@ while true ; do
 	page=1
 	while true ; do
 		ids=$(curl -s "${URL}&page=${page}&since=${since}" \
-			| jq 'try ( .[].payload.patch.id )')
+			| jq "try ( .[].payload.${resource_type}.id )")
 		[ -z "$(echo $ids | tr -d '\n')" ] && break
 		for id in $ids ; do
 			callcmd $id
-- 
2.25.1

[dpdk-ci] [PATCH v4 09/10] tools: filter new patchwork IDs by project name

[Ali Alnubani]

If the script doesn't filter by project, it would be fetching patches
from all projects, which we don't want as different projects
require different checks usually. This patch modifies the script so
that it requires a project's name to fetch IDs for.

Example usage:
$ export MAINTAINERS_FILE_PATH=/path/to/dpdk/MAINTAINERS
$ ./tools/poll-pw series DPDK /path/to/last.txt \
    '/path/to/pw_maintainers_cli.py --type series set-pw-delegate $1'

Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
---
Changes in v3:
- Filter by project using jq, since the events API doesn't have a
  'project' parameter.
- Redirect error messages to stderr.

 tools/poll-pw | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/tools/poll-pw b/tools/poll-pw
index 48c9fd0..f4b89c7 100755
--- a/tools/poll-pw
+++ b/tools/poll-pw
@@ -9,7 +9,7 @@ PAUSE_SECONDS=100
 
 print_usage () {
 	cat <<- END_OF_HELP
-	usage: $(basename $0) [OPTIONS] <patch|series> </path/to/last.txt> <command>
+	usage: $(basename $0) [OPTIONS] <patch|series> <project> </path/to/last.txt> <command>
 
 	Poll patchwork and call a command for each new patch/series id.
 	The first date to filter with is read from the specified file.
@@ -31,15 +31,16 @@ while getopts h arg ; do
 	esac
 done
 
-if [ $# -lt 3 ] ; then
+if [ $# -lt 4 ] ; then
 	printf 'missing argument(s)\n\n' >&2
 	print_usage >&2
 	exit 1
 fi
 shift $(($OPTIND - 1))
 resource_type=$1
-since_file=$2
-shift 2
+project=$2
+since_file=$3
+shift 3
 cmd=$*
 
 if [ ! -f "$since_file" ] ; then
@@ -59,6 +60,12 @@ if [ "$resource_type" != "patch" ] & [ "$resource_type" != "series" ] ; then
 	exit 1
 fi
 
+curl -s $URL/projects/ | jq '.[].name' | grep -qi "^\"${project}\"$"
+if [ ! $? -eq 0 ] ; then
+	printf "The project '$project' doesn't exist.\n\n" >&2
+	exit 1
+fi
+
 URL="${URL}/events/?category=${resource_type}-completed"
 
 callcmd () # <patchwork id>
@@ -73,7 +80,8 @@ while true ; do
 	page=1
 	while true ; do
 		ids=$(curl -s "${URL}&page=${page}&since=${since}" \
-			| jq "try ( .[].payload.${resource_type}.id )")
+			| jq "try ( .[] | select( .project.name == \"$project\" ) )" \
+			| jq "try ( .payload.${resource_type}.id )")
 		[ -z "$(echo $ids | tr -d '\n')" ] && break
 		for id in $ids ; do
 			callcmd $id
-- 
2.25.1

[dpdk-ci] [PATCH v4 10/10] tools: skip the IDs we already fetched

[Ali Alnubani]

To avoid calling 'callcmd' more than once for an ID, which can happen
if a Patchwork ID was created between recording 'date_now' and fetching
the API, store the IDs we already fetched in a file and don't run 'callcmd'
again for any ID that exists in the file while keeping the number of
entries less than 1000 in that file.

Signed-off-by: Ali Alnubani <alialnu@nvidia.com>
---
Changes in v4:
- Limit the number of lines in poll_pw_ids_file (Suggested by Thomas Monjalon).

 tools/poll-pw | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tools/poll-pw b/tools/poll-pw
index f4b89c7..dd19016 100755
--- a/tools/poll-pw
+++ b/tools/poll-pw
@@ -66,6 +66,11 @@ if [ ! $? -eq 0 ] ; then
 	exit 1
 fi
 
+poll_pw_ids_file=/tmp/poll_pw_${resource_type}_ids
+if [ ! -f "$poll_pw_ids_file" ] ; then
+	touch $poll_pw_ids_file
+fi
+
 URL="${URL}/events/?category=${resource_type}-completed"
 
 callcmd () # <patchwork id>
@@ -84,11 +89,19 @@ while true ; do
 			| jq "try ( .payload.${resource_type}.id )")
 		[ -z "$(echo $ids | tr -d '\n')" ] && break
 		for id in $ids ; do
+			if grep -q "^${id}$" $poll_pw_ids_file ; then
+				continue
+			fi
 			callcmd $id
+			echo $id >>$poll_pw_ids_file
 		done
 		page=$((page+1))
 	done
 	echo -n $date_now >$since_file
+	# keep only up to 1000 entries in poll_pw_ids_file
+	if [ $(wc -l <$poll_pw_ids_file) -ge 1000 ] ; then
+		echo "$(tail -500 $poll_pw_ids_file)" >$poll_pw_ids_file
+	fi
 	# pause before next check
 	sleep $PAUSE_SECONDS
 done
-- 
2.25.1

Re: [PATCH v4 00/10] Automatic patchwork delegation

[Thomas Monjalon]

> Ali Alnubani (10):
>   tools: rename guess_git_tree script
>   tools: match by tree URL instead of tree name
>   tools: update script usage
>   tools: add functionality for detecting tree maintainers
>   tools: add functionality for setting pw delegates
>   add git-pw to requirements file
>   tools: filter new Patchwork IDs by date
>   tools: support fetching series
>   tools: filter new patchwork IDs by project name
>   tools: skip the IDs we already fetched

Applied with few stylistic changes, thanks.