Hi Iwase,
Looks good to me! No further feedback.
Thanks again for your help with this.
Brad
On 11 April 2017 at 13:00, Iwase Yusuke <[email protected]> wrote:
> Hi Brad,
>
> Thank you for updating your patch!
>
> How about using "finally statement" to restore sys.path as the attach
> patch?
> This change enables to raise original exception and to remove the
> duplicated codes.
>
> Thanks,
> Iwase
>
>
> On 2017年04月10日 14:31, Brad Cowie wrote:
> > 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 <[email protected]
> <mailto:[email protected]>> 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 <
> 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 <[email protected]
> <mailto:[email protected]> <mailto:[email protected] <mailto:
> [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]> <mailto:
> [email protected] <mailto:[email protected]>> <mailto:
> [email protected] <mailto:[email protected]> <mailto:
> [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> <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> <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:
> [email protected]> <mailto:[email protected]
> <mailto:[email protected]>> <mailto:Ryu-devel@lists.
> sourceforge.net <mailto:[email protected]> <mailto:
> [email protected] <mailto:[email protected]>>>
> > > > > 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>> <
> 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
> > > > [email protected] <mailto:Ryu-devel@lists.
> sourceforge.net> <mailto:[email protected] <mailto:
> [email protected]>>
> > > > 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
> > > [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
> >
>
------------------------------------------------------------------------------
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
