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

["lihuisong (C)" <lihuisong@huawei.com>]

在 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@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?
>
>>       client = Client()
>> -    client.getFilepath(file_path)
>> +    client.getFilepath(args.sock_path)
>>       client.register()
>>       client.interactiveMenu(sleep_time)
>> -- 
>> 2.22.0
>>
> .