Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python3 support #192

Open
dschoonwinkel opened this issue Aug 2, 2017 · 21 comments
Open

Python3 support #192

dschoonwinkel opened this issue Aug 2, 2017 · 21 comments

Comments

@dschoonwinkel
Copy link

Hi

Are there any plans to port the POX controller to python 3?

If not, what alternatives can you recommend?

Thank you!

@Davidmeng78

This comment has been minimized.

@MurphyMc
Copy link
Collaborator

I guess I'll finally need to edit the last sentence of the Python 3 question in the POX FAQ.

The answer is that there are still no concrete plans to do it. Some of the roadblocks to doing it have gone away, though I still haven't heard a compelling reason to do it sooner rather than later. Do you have one?

@MurphyMc
Copy link
Collaborator

Just a note that I've been considering this question again. Specifically, I'm considering having fangtooth be the last Python 2.7 version, with the gar branch being based on Python 3. Welcoming input on the subject.

@dnck
Copy link

dnck commented Jun 26, 2019

Just a note that I've been considering this question again. Specifically, I'm considering having fangtooth be the last Python 2.7 version, with the gar branch being based on Python 3. Welcoming input on the subject.

I'm a complete newb to POX, but I'm interested in learning it by offering help in porting to Python3.

@MurphyMc
Copy link
Collaborator

MurphyMc commented Jul 2, 2019

I don't immediately have much time to put into the effort myself, but my last statement still stands: I'm thinking Python 2 POX may be coming to an end.

If you wanted to put some work in on it, the first thing to do would be to take the current fangtooth branch from my fork, and run it through 2to3.

IIRC, one of the first things that needs fixing up is... in the packet library, there are a bunch of .next methods, which 2to3 thinks are iterator-related and changes to __next__, which isn't right. We can probably just change them back, though this may be a good time to have a bigger conversation about those. I think .payload has been preferred over .next for a while now, but there's still probably a lot of places in the POX code itself that uses .next, and there's a nice duality between prev/next but not between prev/payload. Should .prev change too? To what? .container? (Of course, the whole packet library could use a rewrite, but that's a pretty substantial project of its own.)

Another portability question that immediately comes to mind is pxpcap, the C extension for capturing packets. I have a vague memory of doing some work on it a while back that I hoped would make it easier to port to Python 3, but I'm not sure if a Python 3 compile was ever actually attempted.

@gtataranni
Copy link

gtataranni commented Dec 13, 2019

You might want to consider that Python 2 will reach end of life (EOL) on January 1st, 2020.
If possible, I would convert it to Python 3 as first thing, and then later also consider rewrite considerations.
I have started a conversion on my fork, although I am totally new to pox, so there might still be other leftovers coming up.
So far, I manage to bring it up without errors with ./pox.py samples.pretty_log forwarding.l2_learning on Python 3.7.5, but I haven't tested it any further.
Feel free to contribute :)

@MurphyMc
Copy link
Collaborator

MurphyMc commented Dec 14, 2019

I still can't afford to put time on this, so I appreciate that you are.

I agree with you -- due to the EOL of Python 2, we should go as straightforward as possible. So my statement about the "bigger conversation" above should probably wait. That means that the "next" functions in the packet library should stay as "next". Looking at your PR (MurphyMc#2), though, I think you let 2to3 do its thing with them, which is wrong here. They shouldn't be __next__, since they're not iterators. So all those should get rolled back.

Also, have you given building pxpcap in Python 3 a try?

@larabr
Copy link

larabr commented Dec 19, 2019

I've taken over the porting from @gtataranni , and opened a PR on your fork as well MurphyMc#3 .
As mentioned there, to confirm that everything is working fine I need a bit of support/ideas on how to test some modules (I am not familiar with POX). Thanks!

@MurphyMc
Copy link
Collaborator

MurphyMc commented Mar 9, 2020

Just a note that this is moving on my side. The above note that POX gar will be Python 3 would seem to be coming true. With any luck at all, this will happen within a month.

@MurphyMc
Copy link
Collaborator

I've just pushed a first pass at the POX gar branch, which will be the start of Python 3 support.

Thanks go to @gtataranni and @larabr for getting this started. I tried to incorporate all your changes, though I made a bunch of changes, and also laboriously fixed lots of the dumb things that 2to3 does (and also made a bunch of fixes that 2to3 doesn't). I'm not sure how best to credit you. Maybe a note of thanks for starting the Python 3 port in the README, referencing your github names? Or maybe it's time to start an AUTHORS file?

@MurphyMc
Copy link
Collaborator

Any additional fixes / bug reports / PRs to address outstanding issues with the port are heavily encouraged!

@gtataranni
Copy link

I guess I'll be happy with a mention in whatever format you deem most appropriate :)
Contributions won't be missing if bugs will pop up. Thank you also for your help!

@gtataranni
Copy link

@MurphyMc do you think is going to take long before we see this merged upstream?

@MurphyMc
Copy link
Collaborator

MurphyMc commented Sep 3, 2020

There are two issues.

The first is just sort of finishing a couple things off and moving it to the main repo. This shouldn't actually take much time/work, but I'm completely overloaded at the moment and POX isn't currently on my critical path, so I haven't been able to spare the time for it. It's possible it'll enter my critical path again somewhere between November and January, which would certainly give me an excuse to take care of this.

The second part is when it should become the default branch. The default POX branch is supposed to be relatively stable, and I'm not sure the Python 3 version is there yet. There's an argument that maybe the Python 3 version should be put up front ASAP given the end of CPython 2. But PyPy continues to support Python 2.

@gtataranni
Copy link

gtataranni commented Sep 3, 2020 via email

@reyreaud-l
Copy link

Hello @MurphyMc

Do you know what needs to be done ? I can have a look at fixing some python 2 to 3 problems. Looking at the state of the "gar" branch the only things that need change to make all tests pass is the following patch.

Looked like the factory function was not generating the Object it was supposed to and only returned bytes.

From 5be769c122021915e7668259f9410ac314b54fab Mon Sep 17 00:00:00 2001
From: Loic Reyreaud <[email protected]>
Date: Tue, 22 Sep 2020 12:07:30 +0200
Subject: [PATCH] Fix factory class method on IPAddr6 to return the object

Signed-off-by: Loic Reyreaud <[email protected]>
---
 pox/lib/addresses.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pox/lib/addresses.py b/pox/lib/addresses.py
index 054b29c..f68bd1a 100644
--- a/pox/lib/addresses.py
+++ b/pox/lib/addresses.py
@@ -446,7 +446,7 @@ class IPAddr6 (_AddrBase):
     """
     Factory that creates an IPAddr6 from a large integer
     """
-    return bytes( (num >> i) & 0xff for i in range(120,-8,-8) )
+    return cls( bytes((num >> i) & 0xff for i in range(120,-8,-8) ), raw=True)

   def __init__ (self, addr = None, raw = False, network_order = False):
     """
--
2.28.0

Do you know of other issues ?

@MurphyMc
Copy link
Collaborator

@reyreaud-l : Thanks for that patch; it should get incorporated. Can you post it on its own issue or a PR on the MurphyMc fork to make sure it doesn't get lost?

@reyreaud-l and @gtataranni :
I think the remaining issue I know of is that I went in and did my own fairly careful passes of my own in quite a rush, and this meant that other people that contributed to the Python 3 port don't show up in the commit log or (as of now) anywhere at all really -- even though some of their changes may have been used (or at least served to point out places where I ended up fixing things differently). This somehow needs to get rectified so that people that contributed are credited, but I have less than zero time to spend on it at the moment.

@reyreaud-l
Copy link

I opened a PR here MurphyMc#4 with the patch.

I understand the lack of time to work on such things, we all have different obligations. If it is just a matter of updating the list of people who contributed, we can use the gar branch for now https://github.com/MurphyMc/pox/tree/gar

BTW if there are tasks (coding tasks) where you still need help related to python2 to python3, point them so we can see if we can alleviate the burden of maintaining that and get it done ;)

Thanks for the time you take for answering!

@MurphyMc
Copy link
Collaborator

MurphyMc commented Sep 26, 2020

I've taken the current MurphyMc/gar branch and added it here as gar-experimental. Just so it's clear it may not be quite ready for prime time.

I've also made it the default branch, which goes against the POX policy of the default branch being the stable branch, but it seemed warranted because of the death of Python 2.

(I also filed #249, which was something else I'd hoped to do for gar but won't have time for -- this has been the case since carp, I think. 😢 )

@MurphyMc
Copy link
Collaborator

Also, #242 is tagged gar.

@MurphyMc
Copy link
Collaborator

See also: #250

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants