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 EDDDC45481; Mon, 17 Jun 2024 14:13:56 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C5FEB402BE; Mon, 17 Jun 2024 14:13:56 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mails.dpdk.org (Postfix) with ESMTP id CA14B4028B for ; Mon, 17 Jun 2024 14:13:55 +0200 (CEST) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id D1343DA7; Mon, 17 Jun 2024 05:14:19 -0700 (PDT) Received: from [192.168.50.86] (usa-sjc-mx-foss1.foss.arm.com [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 9727E3F64C; Mon, 17 Jun 2024 05:13:54 -0700 (PDT) Message-ID: Date: Mon, 17 Jun 2024 13:13:49 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 7/8] dts: rework interactive shells Content-Language: en-GB To: =?UTF-8?Q?Juraj_Linke=C5=A1?= , dev@dpdk.org Cc: Jeremy Spewock , Paul Szczepanek References: <20240326190422.577028-1-luca.vizzarro@arm.com> <20240530152505.389167-1-luca.vizzarro@arm.com> <20240530152505.389167-8-luca.vizzarro@arm.com> <4e7b345e-0ad5-4ce4-9c39-7a95fe5770c9@pantheon.tech> From: Luca Vizzarro In-Reply-To: <4e7b345e-0ad5-4ce4-9c39-7a95fe5770c9@pantheon.tech> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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 On 06/06/2024 19:03, Juraj Linkeš wrote: >> +class DPDKShell(InteractiveShell, ABC): >> +    """The base class for managing DPDK-based interactive shells. >> + >> +    This class shouldn't be instantiated directly, but instead be >> extended. >> +    It automatically injects computed EAL parameters based on the >> node in the >> +    supplied app parameters. >> +    """ >> + >> +    _node: SutNode > > Same here, better to be explicit with _sut_node. This should not be changed as it's just overriding the type of the parent's attribute. >> +    _app_params: EalParams >> + >> +    _lcore_filter_specifier: LogicalCoreCount | LogicalCoreList >> +    _ascending_cores: bool >> +    _append_prefix_timestamp: bool >> + >> +    def __init__( >> +        self, >> +        node: SutNode, >> +        app_params: EalParams, >> +        privileged: bool = True, >> +        timeout: float = SETTINGS.timeout, >> +        lcore_filter_specifier: LogicalCoreCount | LogicalCoreList = >> LogicalCoreCount(), >> +        ascending_cores: bool = True, >> +        append_prefix_timestamp: bool = True, >> +        start_on_init: bool = True, >> +    ) -> None: >> +        """Overrides >> :meth:`~.interactive_shell.InteractiveShell.__init__`.""" >> +        self._lcore_filter_specifier = lcore_filter_specifier >> +        self._ascending_cores = ascending_cores >> +        self._append_prefix_timestamp = append_prefix_timestamp >> + >> +        super().__init__(node, app_params, privileged, timeout, >> start_on_init) >> + >> +    def _post_init(self): >> +        """Computes EAL params based on the node capabilities before >> start.""" > > We could just put this before calling super().__init__() in this class > if we update path some other way, right? It's probably better to > override the class method (_update_path()) in subclasses than having > this _post_init() method. It's more complicated than this. The ultimate parent (InteractiveShell) is what sets all the common attributes. This needs to happen before compute_eal_params and updating the path with the dpdk folder. This wouldn't be a problem if we were to call super().__init__ at the beginning. But that way we'd lose the ability to automatically start the shell though.