From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 607EC4239F; Tue, 10 Jan 2023 07:25:04 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0085140691; Tue, 10 Jan 2023 07:25:04 +0100 (CET) Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by mails.dpdk.org (Postfix) with ESMTP id 55A3E40689 for ; Tue, 10 Jan 2023 07:25:01 +0100 (CET) Received: from kwepemm600004.china.huawei.com (unknown [172.30.72.53]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4Nrgjm39xSznV9G; Tue, 10 Jan 2023 14:23:24 +0800 (CST) Received: from [10.67.103.231] (10.67.103.231) by kwepemm600004.china.huawei.com (7.193.23.242) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.34; Tue, 10 Jan 2023 14:24:57 +0800 Message-ID: Date: Tue, 10 Jan 2023 14:24:57 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0 Subject: Re: [PATCH 1/2] usertools: use argparse module to get input parameter To: Bruce Richardson CC: , , , , , References: <20230109065547.8819-1-lihuisong@huawei.com> <20230109065547.8819-2-lihuisong@huawei.com> From: "lihuisong (C)" In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.103.231] X-ClientProxiedBy: dggems702-chm.china.huawei.com (10.3.19.179) To kwepemm600004.china.huawei.com (7.193.23.242) X-CFilter-Loop: Reflected X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 在 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 >>>> --- >>>> 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.