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 F40D84237E; Mon, 9 Jan 2023 13:26:45 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 9C3CE40687; Mon, 9 Jan 2023 13:26:45 +0100 (CET) Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) by mails.dpdk.org (Postfix) with ESMTP id D52FF4067C for ; Mon, 9 Jan 2023 13:26:43 +0100 (CET) Received: from kwepemm600004.china.huawei.com (unknown [172.30.72.53]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4NrCnt0fhVzJrLp; Mon, 9 Jan 2023 20:25:22 +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; Mon, 9 Jan 2023 20:26:37 +0800 Message-ID: Date: Mon, 9 Jan 2023 20:26:37 +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: dggems701-chm.china.huawei.com (10.3.19.178) 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 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? > >> client = Client() >> - client.getFilepath(file_path) >> + client.getFilepath(args.sock_path) >> client.register() >> client.interactiveMenu(sleep_time) >> -- >> 2.22.0 >> > .