Re: [dpdk-dev] [PATCH v10 2/3] devtools: script to send notifications of expired symbols

["Kinsella, Ray" <mdr@ashroe.eu>]

On 01/09/2021 13:46, Aaron Conole wrote:
> Ray Kinsella <mdr@ashroe.eu> writes:
> 
>> Use this script with the output of the DPDK symbol tool, to notify
>> maintainers of expired symbols by email. You need to define the environment
>> variable DPDK_GETMAINTAINER_PATH for this tool to work.
>>
>> Use terminal output to review the emails before sending.
>> e.g.
>> $ devtools/symbol-tool.py list-expired --format-output csv \
>> | DPDK_GETMAINTAINER_PATH=<somewhere>/get_maintainer.pl \
>> devtools/notify_expired_symbols.py --format-output terminal
>>
>> Then use email output to send the emails to the maintainers.
>> e.g.
>> $ devtools/symbol-tool.py list-expired --format-output csv \
>> | DPDK_GETMAINTAINER_PATH=<somewhere>/get_maintainer.pl \
>> devtools/notify_expired_symbols.py --format-output email \
>> --smtp-server <server> --sender <someone@somewhere.com> \
>> --password <password> --cc <someone@somewhere.com>
>>
>> Signed-off-by: Ray Kinsella <mdr@ashroe.eu>
>> ---
>>  devtools/notify-symbol-maintainers.py | 256 ++++++++++++++++++++++++++
>>  1 file changed, 256 insertions(+)
>>  create mode 100755 devtools/notify-symbol-maintainers.py
>>
>> diff --git a/devtools/notify-symbol-maintainers.py b/devtools/notify-symbol-maintainers.py
>> new file mode 100755
>> index 0000000000..ee554687ff
>> --- /dev/null
>> +++ b/devtools/notify-symbol-maintainers.py
>> @@ -0,0 +1,256 @@
>> +#!/usr/bin/env python3
>> +# SPDX-License-Identifier: BSD-3-Clause
>> +# Copyright(c) 2021 Intel Corporation
>> +# pylint: disable=invalid-name
>> +'''Tool to notify maintainers of expired symbols'''
>> +import smtplib
>> +import ssl
>> +import sys
>> +import subprocess
>> +import argparse
>> +from argparse import RawTextHelpFormatter
>> +import time
>> +from email.message import EmailMessage
>> +
>> +DESCRIPTION = '''
>> +Use this script with the output of the DPDK symbol tool, to notify maintainers
>> +and contributors of expired symbols by email. You need to define the environment
>> +variable DPDK_GETMAINTAINER_PATH for this tool to work.
>> +
>> +Use terminal output to review the emails before sending.
>> +e.g.
>> +$ devtools/symbol-tool.py list-expired --format-output csv \\
>> +| DPDK_GETMAINTAINER_PATH=<somewhere>/get_maintainer.pl \\
>> +{s} --format-output terminal
>> +
>> +Then use email output to send the emails to the maintainers.
>> +e.g.
>> +$ devtools/symbol-tool.py list-expired --format-output csv \\
>> +| DPDK_GETMAINTAINER_PATH=<somewhere>/get_maintainer.pl \\
>> +{s} --format-output email \\
>> +--smtp-server <server> --sender <someone@somewhere.com> --password <password> \\
>> +--cc <someone@somewhere.com>
>> +'''
>> +
>> +EMAIL_TEMPLATE = '''Hi there,
>> +
>> +Please note the symbols listed below have expired. In line with the DPDK ABI
>> +policy, they should be scheduled for removal, in the next DPDK release.
>> +
>> +For more information, please see the DPDK ABI Policy, section 3.5.3.
>> +https://doc.dpdk.org/guides/contributing/abi_policy.html
>> +
>> +Thanks,
>> +
>> +The DPDK Symbol Bot
>> +
>> +'''
>> +
>> +ABI_POLICY = 'doc/guides/contributing/abi_policy.rst'
>> +MAINTAINERS = 'MAINTAINERS'
>> +get_maintainer = ['devtools/get-maintainer.sh', \
>> +                  '--email', '-f']
> 
> Maybe it's best to make this something that can be overridden.  There's
> a series to change the .sh files to .py files.  Perhaps an environment
> variable or argument?
> 
>> +def _get_maintainers(libpath):
>> +    '''Get the maintainers for given library'''
>> +    try:
>> +        cmd = get_maintainer + [libpath]
>> +        result = subprocess.run(cmd, \
>> +                                stdout=subprocess.PIPE, \
>> +                                stderr=subprocess.PIPE,
>> +                                check=True)
>> +    except subprocess.CalledProcessError:
>> +        return None
> 
> You might consider handling
> 
>    except FileNotFoundError:
>       ....
> 
> With a graceful exit and error message.  In case the get_maintainers
> path changes.
> 

So FYI - this get's into the weed a bit.
As there is already a DPDK_GETMAINTAINER_PATH environment variable,
what would you call a new variable.

So instead I added logic for the script to sanity check that _everything_
is defined and where it expects it to be, and then complain loudly 
and die when it is not.

The devtools scripts already cross-reference either each, so I'd expect
any changes changing to get-maintainers.sh to get-maintainers.py to take
care of cross-references. 

Ray K