[PATCH 1/2] usertools: use argparse module to get input parameter

[lihuisong (C)]

在 2023/1/9 22:32, Bruce Richardson 写道:
> On Mon, Jan 09, 2023 at 08:26:37PM +0800, lihuisong (C) wrote:
>> 在 2023/1/9 17:14, Bruce Richardson 写道:
>>> On Mon, Jan 09, 2023 at 02:55:46PM +0800, Huisong Li wrote:
>>>> The telemetry client script uses argparse module to get input parameter.
>>>>
>>>> Signed-off-by: Huisong Li <lihuisong at huawei.com>
>>>> ---
>>>>    usertools/dpdk-telemetry-client.py | 14 +++++++-------
>>>>    1 file changed, 7 insertions(+), 7 deletions(-)
>>>>
>>> This is an old script using the older telemetry V1 interface, so I'd
>>> generally recommend users switch to using scripts for the v2 interface.
>>> That said, no reason not to improve the script while we have it.
>> Yes. After all, the telemetry v1 interface and this script are still exist.
>>>> diff --git a/usertools/dpdk-telemetry-client.py b/usertools/dpdk-telemetry-client.py
>>>> index df41d04fbe..fd69955b32 100755
>>>> --- a/usertools/dpdk-telemetry-client.py
>>>> +++ b/usertools/dpdk-telemetry-client.py
>>>> @@ -6,6 +6,7 @@
>>>>    import os
>>>>    import sys
>>>>    import time
>>>> +import argparse
>>>>    BUFFER_SIZE = 200000
>>>> @@ -115,13 +116,12 @@ def interactiveMenu(self, sleep_time): # Creates Interactive menu within the scr
>>>>    if __name__ == "__main__":
>>>>        sleep_time = 1
>>>> -    file_path = ""
>>>> -    if len(sys.argv) == 2:
>>>> -        file_path = sys.argv[1]
>>>> -    else:
>>>> -        print("Warning - No filepath passed, using default (" + DEFAULT_FP + ").")
>>>> -        file_path = DEFAULT_FP
>>>> +    parser = argparse.ArgumentParser()
>>>> +    parser.add_argument('-s', '--sock_path', default=DEFAULT_FP,
>>>> +                        help='Provide socket file path connected by legacy client')
>>>> +    args = parser.parse_args()
>>>> +
>>> While I like using argparse rather than handling args directly, this breaks
>>> compatibility.  For anyone already using this script via automation, this
>>> would break things, as the path needs to be provided via a "-s" parameter,
>>> rather than just tacked on as argv[1].
>> If there isn't the modification patch 2/2 mentioned, this script cannot be
>> directly used in most scenarios. From the first commit of this script, it's
>> just used as a demo client example. See
>> commit d1b94da4a4e0 ("usertools: add client script for telemetry")
>>  From this point of view, can this compatibility issue be ignored?
> Agree with you that patch 2/2 is necessary to make script useful for most
> cases, and also agree that argparse is the better way to do argument
> handling. However, I also think that we can keep compatibility in this
> matter - you can add optional positional arguments in argparse to support
> backward compatibility.
>
> 	parser.add_argument('sock_path', nargs='?', default=...)
>
> should work, I believe, for this case.
It works well. Thank you for your suggestion. I will fix it in v2.