Hi Iwase,
Thanks for the followup.
In testing I found a few issues with your patch and have some feedback:
- Windows style line endings
- Breaks python2.6 & python2.7 compatibility (FileNotFoundError is python3
only)
- Doesn't update sys.path to include path to python file which causes
issues with imports
I have written an updated patch and attached that fixes these issues and
further reworks the loading behaviour of Ryu.
What I've done is switched from using exceptions to an if statement to
decide on how a given RyuApp should be loaded. The reason I've done this is
because imp.load_source and importlib will pass exceptions from the user
code up the stack to Ryu which means that an exception inside the RyuApp
would cause us to do a module import even though we already loaded the
RyuApp from a file.
Let me know what you think of this patch.
Brad
On 7 April 2017 at 19:56, Iwase Yusuke <[email protected]> wrote:
> Hi Brad,
>
> Sorry for the delay.
>
> I make a patch for this issue.
> Could you test the attached patch?
>
> Thanks,
> Iwase
>
>
> On 2017年03月24日 17:19, Brad Cowie wrote:
> > Hi Iwase,
> >
> > Yes I suspect most people will prefer that when a file path is specified
> as a ryu app that file path given is preferred over loading a module.
> >
> > Perhaps an additional check - if the app given to ryu-manager exists as
> a regular file, load that, otherwise load the module by the same name?
> >
> > Brad
> >
> > On 24 March 2017 at 19:46, Iwase Yusuke <[email protected]
> <mailto:[email protected]>> wrote:
> >
> > Hi Brad,
> >
> > Thank you for your reporting!
> >
> > Yes, as you said, ryu.utils.import_module() prefers to loading a
> Python module than file paths.
> > In order to solve this problem, we need to change the priority
> (Python module or file path), I guess.
> >
> > You mean "file path should be preferred", right?
> >
> > Thanks,
> > Iwase
> >
> >
> > On 2017年03月19日 11:05, Brad Cowie wrote:
> > > Hi there,
> > >
> > > If you have a RyuApp and put it in a python file that happens to
> have the same name as any python module installed in PYTHON_PATH,
> ryu-manager will be unable to load the RyuApp.
> > >
> > > Here is an example showing the issue in ryu-manager 4.12:
> > >
> > > brad@zilch ~/D/r/r/app> ryu-manager ./simple_switch_13.py
> > > loading app ./simple_switch_13.py
> > > loading app ryu.controller.ofp_handler
> > > instantiating app ./simple_switch_13.py of SimpleSwitch13
> > > instantiating app ryu.controller.ofp_handler of OFPHandler
> > > [loads fine]
> > >
> > > brad@zilch ~/D/r/r/app> pip freeze | grep six
> > > six==1.10.0
> > > [find name of any package installed with pip]
> > >
>
>
> brad@zilch ~/D/r/r/app> mv
> ./simple_switch_14.py ./six.py
> > > brad@zilch ~/D/r/r/app> ryu-manager ./six.py
> > > loading app ./six.py
> > > [ryu-manager will attempt to load a RyuApp from
> /usr/lib/python2.7/dist-packages/six.py instead of ./six.py]
> > >
> > > This seems to be an issue in ryu <https://github.com/osrg/ryu/
> tree/master/ryu <https://github.com/osrg/ryu/tree/master/ryu>>/*utils.py*
> where import_module() will prefer to load a python module even when
> presented with a relative or absolute file path.
> > >
> > > Interested on people's thoughts on the best way to solve this one.
> > >
> > > Regards,
> > > Brad
> > >
> > >
> > > ------------------------------------------------------------
> ------------------
> > > Check out the vibrant tech community on one of the world's most
> > > engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> > >
> > >
> > >
> > > _______________________________________________
> > > Ryu-devel mailing list
> > > [email protected] <mailto:Ryu-devel@lists.
> sourceforge.net>
> > > https://lists.sourceforge.net/lists/listinfo/ryu-devel <
> https://lists.sourceforge.net/lists/listinfo/ryu-devel>
> > >
> >
> >
> >
> >
> > ------------------------------------------------------------
> ------------------
> > Check out the vibrant tech community on one of the world's most
> > engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> >
> >
> >
> > _______________________________________________
> > Ryu-devel mailing list
> > [email protected]
> > https://lists.sourceforge.net/lists/listinfo/ryu-devel
> >
>
From 0dbca830cbc52a3af37a41ec75ed50c2c7004bf7 Mon Sep 17 00:00:00 2001
From: Brad Cowie <[email protected]>
Date: Sat, 8 Apr 2017 14:05:45 +1200
Subject: [PATCH] utils.import_module: Prefer filepath than Python module
Currently, ryu.utils.import_module() prefers the Python module path
than the user specified file path, and if the user app name is the
same with the Python module, this function returns the Python module
instead of the user app.
This patch fixes to try to load "file" before finding the loaded
Python module, and fixes this problem.
Note: With this patch, import_module() will reload (override) a module
if the specified module file name is the same.
---
ryu/tests/unit/lib/test_import_module.py | 5 ++--
ryu/utils.py | 41 ++++++++++++++++----------------
2 files changed, 22 insertions(+), 24 deletions(-)
diff --git a/ryu/tests/unit/lib/test_import_module.py b/ryu/tests/unit/lib/test_import_module.py
index 25264c3..b8561d2 100644
--- a/ryu/tests/unit/lib/test_import_module.py
+++ b/ryu/tests/unit/lib/test_import_module.py
@@ -44,9 +44,8 @@ class Test_import_module(unittest.TestCase):
eq_("this is ccc", ccc.name)
ddd = import_module('./lib/test_mod/ddd/mod.py')
# Note: When importing a module by filename, if module file name
- # is duplicated, import_module returns a module instance which is
- # imported before.
- eq_("this is ccc", ddd.name)
+ # is duplicated, import_module reload (override) a module instance.
+ eq_("this is ddd", ddd.name)
def test_import_same_module1(self):
from ryu.tests.unit.lib.test_mod import eee as eee1
diff --git a/ryu/utils.py b/ryu/utils.py
index 44d6bf3..387d92f 100644
--- a/ryu/utils.py
+++ b/ryu/utils.py
@@ -80,29 +80,28 @@ def _find_loaded_module(modpath):
return None
+def _import_module_file(path):
+ abspath = os.path.abspath(path)
+ sys.path.append(os.path.dirname(abspath))
+ modname = chop_py_suffix(os.path.basename(abspath))
+ return load_source(modname, abspath)
+
+
def import_module(modname):
- try:
- # Import module with python module path
- # e.g.) modname = 'module.path.module_name'
+ if os.path.exists(modname):
+ try:
+ # Try to import module since 'modname' is a valid path to a file
+ # e.g.) modname = './path/to/module/name.py'
+ return _import_module_file(modname)
+ except SyntaxError:
+ # The file didn't parse as valid Python code, try
+ # importing module assuming 'modname' is a Python module name
+ # e.g.) modname = 'path.to.module.name'
+ return importlib.import_module(modname)
+ else:
+ # Import module assuming 'modname' is a Python module name
+ # e.g.) modname = 'path.to.module.name'
return importlib.import_module(modname)
- except (ImportError, TypeError):
- # In this block, we retry to import module when modname is filename
- # e.g.) modname = 'module/path/module_name.py'
- abspath = os.path.abspath(modname)
- # Check if specified modname is already imported
- mod = _find_loaded_module(abspath)
- if mod:
- return mod
- # Backup original sys.path before appending path to file
- original_path = list(sys.path)
- sys.path.append(os.path.dirname(abspath))
- # Remove python suffix
- name = chop_py_suffix(os.path.basename(modname))
- # Retry to import
- mod = importlib.import_module(name)
- # Restore sys.path
- sys.path = original_path
- return mod
def round_up(x, y):
--
2.7.4
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Ryu-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ryu-devel
