test suite reviews and discussions
 help / color / mirror / Atom feed
From: "Juraj Linkeš" <juraj.linkes@pantheon.tech>
To: "Juraj Linkeš" <juraj.linkes@pantheon.tech>,
	"Tu, Lijuan" <lijuan.tu@intel.com>,
	"ohilyard@iol.unh.edu" <ohilyard@iol.unh.edu>
Cc: "dts@dpdk.org" <dts@dpdk.org>
Subject: Re: [dts] [PATCH v2] python: standard project structure
Date: Fri, 10 Sep 2021 12:31:24 +0000	[thread overview]
Message-ID: <aba9e5ec221d450c83fc58796f9edf1f@pantheon.tech> (raw)
In-Reply-To: <ccc012c28c3b4313a94ca9fec364a2fc@pantheon.tech>



> -----Original Message-----
> From: dts <dts-bounces@dpdk.org> On Behalf Of Juraj Linkeš
> Sent: Thursday, September 9, 2021 1:05 PM
> To: Tu, Lijuan <lijuan.tu@intel.com>; ohilyard@iol.unh.edu
> Cc: dts@dpdk.org
> Subject: Re: [dts] [PATCH v2] python: standard project structure
> 
> 
> 
> > -----Original Message-----
> > From: Tu, Lijuan <lijuan.tu@intel.com>
> > Sent: Monday, September 6, 2021 5:01 AM
> > To: Juraj Linkeš <juraj.linkes@pantheon.tech>; ohilyard@iol.unh.edu
> > Cc: dts@dpdk.org
> > Subject: RE: [PATCH v2] python: standard project structure
> >
> > > -----Original Message-----
> > > From: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > > Sent: 2021年8月27日 21:04
> > > To: Tu, Lijuan <lijuan.tu@intel.com>; ohilyard@iol.unh.edu
> > > Cc: dts@dpdk.org; Juraj Linkeš <juraj.linkes@pantheon.tech>
> > > Subject: [PATCH v2] python: standard project structure
> > >
> > > DTS does not use the standard project structure and has therefore
> > > add paths to python interpreter search paths. Move to a standard structure:
> > > * Move main.py to the root directory.
> > > * Add __init__.py to directories from which python modules are imported.
> > > * Adjust import paths to account for this new structure.
> > >
> > > Moving to a standard structure has a host of positives, such as:
> > > * No need to study any non-standard approaches. This removes any
> > > bewilderment the developers may feel when encountering something
> > > non-standard without proper justification (as is the case with DTS).
> > > * Better integration with IDEs which rely on the standard structure.
> > > * More accurate results from automated tools.
> > >
> > > In addition to this, not only adjust the import paths but make then
> > > explicit for modules imported from the same level, e.g. instead of
> > > using from foo import, use from .foo import (and import bar.foo as
> > > foo, where foo if on the same level as the importing module). This
> > > allows developers to separate DTS modules from third party modules
> > > at a
> > glance.
> > >
> > > Also sort the import using the isort tool. Add config using the "black"
> > > profile for isort to facilitate interoperability with Black.
> > >
> > > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > > ---
> > > v2:
> > > * Reintroduced the dts shell cript.
> > > * Tested the imports by trying to import everything and fixed what I
> > > * found.
> > > * Sorted the imports using isort with the black profile.
> > > ---
> >
> > * the import of Dot1BR  is broken.
> 
> I don't understand how this works exactly. Are there two distinct tools that are
> trying to use dep/scapy_modules/Dot1BR.py?
> 
> > 	Dot1BR  is aimed to import in scapy application in Tester.
> 
> Is this something that runs outside of DTS? Doesn't DTS import Scapy libs and
> send packets using those libs? What exactly is the scapy application and how it's
> supposed to used dep/scapy_modules/Dot1BR.py?
> 
> > 	The patch just import it in the script, but not in the scapy.
> 
> What script are you talking about here? Maybe diagram would be helpful - what
> runs where and what is supposed to import what.
> 

I think I figured out what the problem is - we need SCAPY_IMP_CMD to include dep.scapy_modules.Dot1BR since this is used to interactively import the libraries using expect.

The interactive use of interpreter is dubious and I'd like to see some other approach (such as just running a script) instead. But that's another topic so I'll put back Dot1BR to the list of interactively imported modules and just make some changes that will make, in my opinion, the code a bit more readable.

> > 	So that when sending packet by scapy, it can't recognize Dot1BR
> > packet.
> >
> > * Suite directory is still imported, which should by remove.
> >    - File: framework/dts.py  line: 534
> > 	# add python module search path
> >     	sys.path.append(suite_dir)
> >
> 
> I'll remove this, but that will make the  "--suite-dir" argument obsolete. We
> should think about how to deprecate it - if we decide to do that, it should be in
> another patch though. I'd guess it would a very low priority.
> 
> > * Missed some dynamic import in the function:
> >    - File: framework/dts.py
> > 	line: 250
> > 		project_module = __import__(PROJECT_MODULE_PREFIX +
> > project_name)
> > 	line: 454
> > 		suite_module = __import__('TestSuite_' + suite_name)
> >    - File: framework/test_case.py		line: 380
> > 	debugger.AliveModule = __import__('TestSuite_' + self.suite_name)
> >    - File:	framework/tester.py	line: 740, 846, 855
> > 	module = __import__("packet")
> >    - File: framework/flow/flow_pattern_item.py 	line: 92
> > 	from flow import Flow
> >
> 
> I also found some other instances of __import__ being called (in tester.py and
> dump_case.py), I'll fix all of them.
> 
> We may want to replace __import__() with importlib.import_module(), as the
> docs say that direct use of __import__() is discouraged (in favor of
> importlib.import_module()). This would also be a low priority item I think.

  reply	other threads:[~2021-09-10 12:31 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-24 11:34 [dts] [PATCH v1] " Juraj Linkeš
2021-08-24 13:46 ` Owen Hilyard
2021-08-27  9:19   ` Juraj Linkeš
2021-08-27 13:03 ` [dts] [PATCH v2] " Juraj Linkeš
2021-09-06  3:01   ` Tu, Lijuan
2021-09-09 11:05     ` Juraj Linkeš
2021-09-10 12:31       ` Juraj Linkeš [this message]
2021-09-10 13:41   ` [dts] [PATCH v3] " Juraj Linkeš
2021-09-16  2:33     ` Tu, Lijuan
2021-10-07 11:26     ` [dts] [PATCH v4] " Juraj Linkeš
2021-10-19 12:51       ` [dts] [PATCH v5] " Juraj Linkeš
2021-10-22  8:28         ` Tu, Lijuan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aba9e5ec221d450c83fc58796f9edf1f@pantheon.tech \
    --to=juraj.linkes@pantheon.tech \
    --cc=dts@dpdk.org \
    --cc=lijuan.tu@intel.com \
    --cc=ohilyard@iol.unh.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).