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 3CF30A0C4D; Tue, 24 Aug 2021 15:47:13 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 01516406A3; Tue, 24 Aug 2021 15:47:13 +0200 (CEST) Received: from mail-ot1-f54.google.com (mail-ot1-f54.google.com [209.85.210.54]) by mails.dpdk.org (Postfix) with ESMTP id 0C0FB40687 for ; Tue, 24 Aug 2021 15:47:12 +0200 (CEST) Received: by mail-ot1-f54.google.com with SMTP id 61-20020a9d0d430000b02903eabfc221a9so46252887oti.0 for ; Tue, 24 Aug 2021 06:47:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=+BYN+/nAmRdsYJ+rXkrljoiyDVQvcPEFEiCGFs4NoxY=; b=Kl8+R04M5hzP0YMrtBoKdh0nJF2xZdlcMGrDQPr5k0KHfAMb/6PnnwTr8MXTmXDayY vGtXDQJU5MpL7BF4M+VvDkRG4mLWvSqei4prCYOVT76eVQNXol/IAXpMWdDpypKPiPPt SHXM38B1cukRRqhBacX8kpbbNLQtC6ZMHo7lc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=+BYN+/nAmRdsYJ+rXkrljoiyDVQvcPEFEiCGFs4NoxY=; b=CTk2GtKWjv/SseVpz1en0Aep93l0IJC5HKUMVNq4F/KiXmMMCARsvdDTruKBAzaNbs +0jDEnYylBo7Zwfjpkl/WWUJBaNvzJPgIaMdDnxD6PEPurp0Tot/22w1D+hQ+n9PCTYG GXIHLJeQkDONmn/Yuj+OnWA93ZbWOr6QOEhvkpuR/t0Q5pzqw76+hsogFUFmV3cNInaC PcZKgt2BFENNF4S4dWjCgH/BzLVLo8BlyMY0zQEAxiRc9MjpkGyV79giDQRYXgVseXDc n2hVWd86nuM8w6SITX4V2q/WSiFJfkAwBePJvzu7FHocnyk2vp4ic2EmF1VFeb4NiSAb 5Hsw== X-Gm-Message-State: AOAM532aY3muD9atNaNU+Jh6CG9dFYbNnY0M9EO417kSGFcd2kmV6Fr5 xix02vv7VgPROsDeaz8qst9b84eHMzJeGBh+y3GilUTz+U/X3g== X-Google-Smtp-Source: ABdhPJyf0a3kAKYb8CgxGQECVAPO8o/HGvrTNzztLXoqpsxJiiNWnhF19EyfPBoskweeWtOfmXDkJ0cs7iyqEMOipZI= X-Received: by 2002:aca:4e08:: with SMTP id c8mr2958799oib.79.1629812830710; Tue, 24 Aug 2021 06:47:10 -0700 (PDT) MIME-Version: 1.0 References: <1629804878-11074-1-git-send-email-juraj.linkes@pantheon.tech> In-Reply-To: <1629804878-11074-1-git-send-email-juraj.linkes@pantheon.tech> From: Owen Hilyard Date: Tue, 24 Aug 2021 09:46:34 -0400 Message-ID: To: =?UTF-8?Q?Juraj_Linke=C5=A1?= Cc: "Tu, Lijuan" , dts@dpdk.org Content-Type: multipart/alternative; boundary="000000000000d38c1b05ca4e5e77" Subject: Re: [dts] [PATCH v1] python: standard project structure X-BeenThere: dts@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: test suite reviews and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dts-bounces@dpdk.org Sender: "dts" --000000000000d38c1b05ca4e5e77 Content-Type: text/plain; charset="UTF-8" > > (and consequently remove the redundant dts shell script). I don't think we should be making a breaking change to the external interface right now. It may be worth it to discuss adding a deprecation warning to the script, but we should give time for anyone who is using DTS to using python directly. This includes the Community CI Lab, since this change would break our current approach to updating DTS, which is to have a "canary" system updated and running for a bit to find potential issues before everything else gets updated. 1. Should we do the explicit import paths? It may be easier to force absolute import paths since I know some older automated tooling might now handle relative imports correctly. They also make refactoring much easier if we ever need to move files around again. > 2. If so, do we want to enforce it (for new patches)? How (is there a tool > that check that or just manually in review)? I think we should just to keep everything consistent throughout the codebase. I don't know if there is a tool, but it should be easy to look at the top of files in a patch and check for relative imports. > 3. Some paths might not have been moved to the explicit format, as I > may have missed them. > 4. I did not test the patch as I don't have access to an environment > in which I could do so. Is there a way to test it locally (i.e. without > any hw dependencies)? The easiest way would probably be to put something like this in main to test: from framework import * from framework.flow import * from framework.ixia_network import * from tests import * from nics import * This should import every single file in the project, which will cause every file to evaluate its own imports. As long as nothing imports main, that should work. 5. Do we want to sort imports? I think that we should ask that imports are logically grouped, but not prescribe any particular method unless we start to have issues. > 6. If so, how? Do we want to do it in this patch? It might make sense to do a single pass now using isort ( https://github.com/PyCQA/isort). We can discuss adding that as a tool later, but unless there's an easy way to add it as a commit hook, it might make sense to do it once and leave it be. DTS has a lot of other stuff to work on, and I think that this is something we can discuss once we've fixed some of the bigger issues. --000000000000d38c1b05ca4e5e77 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
(and con= sequently remove the=C2=A0redundant dts shell script).
I don't think we should be making a breaking change to the = external interface right now. It may be worth it to discuss adding a deprec= ation warning to the script, but we should give time for anyone who is usin= g DTS to using python directly. This includes the Community CI Lab, since t= his change would break our current approach to updating DTS, which is to ha= ve a "canary" system updated and running for a bit to find potent= ial issues before everything else gets updated.

1. Should we do the explici= t import paths?

It may be easier to force a= bsolute import paths since I know some older automated tooling might now ha= ndle relative imports correctly. They also make refactoring much easier if = we ever need to move files around again.
=C2=A0
2. If so, do we want to enforce it (f= or new patches)? How (is there a=C2=A0tool that check that or just manually= in review)?

I think we should just=C2=A0to= keep everything consistent throughout the codebase. I don't know if th= ere is a tool, but it should be easy to look at the top of files in a patch= and check for relative imports.=C2=A0
=C2=A0
3. Some paths might not have been moved= to the explicit format, as I may=C2=A0have missed them.
4. I did not te= st the patch as I don't have access to an environment in=C2=A0which I c= ould do so. Is there a way to test it locally (i.e. without any=C2=A0hw dep= endencies)?

The easiest way would probably = be to put something like this in main to test:

from framework import *
from = framework.flow import *
from fram= ework.ixia_network import *
from = tests import *
from nics import *=
=C2=A0
This should import every single file in = the project, which will cause every file to evaluate=C2=A0its own imports. = As long as nothing imports main, that should work.=C2=A0

5. Do we want to sort i= mports?

I think that we should ask that imp= orts are logically grouped, but not prescribe any particular method unless = we start to have issues.=C2=A0
=C2=A0
6. If so, how? Do we want to do it in this patc= h?
=C2=A0
It might make sense to do a sing= le pass now using isort (https:/= /github.com/PyCQA/isort). We can discuss adding that as a tool later, b= ut unless there's an easy way to add it as a commit hook, it might make= sense to do it once and leave it be. DTS has a lot of other stuff to work = on, and I think that this is something we can discuss once we've fixed = some of the bigger issues.=C2=A0
--000000000000d38c1b05ca4e5e77--