Hi Iwase,
You are quite right I forgot to copy that part of the code over.
I have attached a refreshed patch that restores sys.path after a file is
imported.
Let me know what you think!
Brad
On 10 April 2017 at 14:22, Iwase Yusuke <iwase.yusu...@gmail.com> wrote:
> Hi Brad,
>
> Great! Thanks a lot!
> It looks almost good to me.
>
> A few remarks, if update sys.path before importing module file, I guess we
> need to backup
> the original sys.path and restore it before returning module instance as
> following.
> https://github.com/osrg/ryu/blob/master/ryu/utils.py#L96-L97
> This avoids inserting the unexpected path into sys.path.
>
> Thanks,
> Iwase
>
>
> On 2017年04月08日 12:25, Brad Cowie wrote:
> > 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 <iwase.yusu...@gmail.com <mailto:
> iwase.yusu...@gmail.com>> 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 <iwase.yusu...@gmail.com
> <mailto:iwase.yusu...@gmail.com> <mailto:iwase.yusu...@gmail.com <mailto:
> iwase.yusu...@gmail.com>>> 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> <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
> > > > Ryu-devel@lists.sourceforge.net <mailto:Ryu-devel@lists.
> sourceforge.net> <mailto:Ryu-devel@lists.sourceforge.net <mailto:
> Ryu-devel@lists.sourceforge.net>>
> > > > https://lists.sourceforge.net/lists/listinfo/ryu-devel <
> https://lists.sourceforge.net/lists/listinfo/ryu-devel> <
> 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
> > > Ryu-devel@lists.sourceforge.net <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
> > Ryu-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/ryu-devel
> >
>
From 71a05d0325c3a8fb61bc1f65607ea91d328aeee0 Mon Sep 17 00:00:00 2001
From: Brad Cowie <b...@gizmoguy.net.nz>
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.
Signed-off-by: Brad Cowie <b...@gizmoguy.net.nz>
---
ryu/tests/unit/lib/test_import_module.py | 5 ++--
ryu/utils.py | 51 +++++++++++++++++++-------------
2 files changed, 32 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..63dbcc8 100644
--- a/ryu/utils.py
+++ b/ryu/utils.py
@@ -80,29 +80,38 @@ def _find_loaded_module(modpath):
return None
-def import_module(modname):
+def _import_module_file(path):
+ abspath = os.path.abspath(path)
+ # Backup original sys.path before appending path to file
+ original_path = list(sys.path)
+ sys.path.append(os.path.dirname(abspath))
+ modname = chop_py_suffix(os.path.basename(abspath))
try:
- # Import module with python module path
- # e.g.) modname = 'module.path.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
+ mod = load_source(modname, abspath)
+ except:
+ # Restore sys.path on exception
sys.path = original_path
- return mod
+ raise
+ # Restore sys.path before returning
+ sys.path = original_path
+ return mod
+
+
+def import_module(modname):
+ 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)
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
Ryu-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ryu-devel
