Merge Request Review Reminder
Hello, I hope you are all well, I currently have an issue / merge request that has become stale and as per the guidelines I am sending this e-mail to request someone to review it for me please. Issue Number: 42861 (https://bugs.python.org/issue42861) Pull Request: 24180 (https://github.com/python/cpython/pull/24180) This is my first contribution to CPython and I am very keen to get this added as I think it is an almost essential enhancement missing from the ipaddress module. I would greatly appreciate if someone can take a look and give me feedback, I am more than happy to help explain things, as I appreciate this is not the most straightforward thing to do. Effectively, this method allows you to get the next closest network of any prefix size, it works by effectively adding 1 to the host portion of the network. There is actually a very nice guide someone made on stackexchange explaining how to do this calculation manually, I simply followed this logic and have tested it as much as possible and it seems to work great. See here: https://networkengineering.stackexchange.com/questions/7106/how-do-you-calcu... Please let me know if there is anything else I can do to help get this merged. Kind regards, Faisal.
Hello, Following my previous e-mail last month, thank you for responding. I almost immediately got two reviewers who posted helpful comments on my PR which I believe have all been resolved now. Today, I got a notification to say my branch is stale again so wanted to understand what the next steps are. Is there anything else I can do to get my PR merged? This is my first time submitting a PR to Cpython so not sure of the workflow, or if I have missed something so would appreciate your guidance. SIDE QUESTION: Also, when I originally submitted this PR, I used a different Git config and user called "KillerKode" - however I was asked to change this, which is totally understandable so I changed everything to my name, however the commit history of that PR now contains some commits from my old Git config (as KillerKode) and some with my new Git config (as Faisal Mahmood). I am wondering, when this gets merged, how can I make sure that the commits are squashed and any history of my previous git config (i.e. KillerKode) do not show? Kind regards, Faisal. On Mon, 15 Feb 2021 at 14:20, Faisal Mahmood <fasial.mahmood94@gmail.com> wrote:
Hello,
I hope you are all well, I currently have an issue / merge request that has become stale and as per the guidelines I am sending this e-mail to request someone to review it for me please.
Issue Number: 42861 (https://bugs.python.org/issue42861) Pull Request: 24180 (https://github.com/python/cpython/pull/24180)
This is my first contribution to CPython and I am very keen to get this added as I think it is an almost essential enhancement missing from the ipaddress module. I would greatly appreciate if someone can take a look and give me feedback, I am more than happy to help explain things, as I appreciate this is not the most straightforward thing to do.
Effectively, this method allows you to get the next closest network of any prefix size, it works by effectively adding 1 to the host portion of the network. There is actually a very nice guide someone made on stackexchange explaining how to do this calculation manually, I simply followed this logic and have tested it as much as possible and it seems to work great. See here: https://networkengineering.stackexchange.com/questions/7106/how-do-you-calcu...
Please let me know if there is anything else I can do to help get this merged.
Kind regards,
Faisal.
For the side question, you can always use rebase instead of merge. Let me know if you need further particulars? -- H On Fri, 19 Mar 2021 at 09:31, Faisal Mahmood <fasial.mahmood94@gmail.com> wrote:
Hello,
Following my previous e-mail last month, thank you for responding. I almost immediately got two reviewers who posted helpful comments on my PR which I believe have all been resolved now.
Today, I got a notification to say my branch is stale again so wanted to understand what the next steps are.
Is there anything else I can do to get my PR merged? This is my first time submitting a PR to Cpython so not sure of the workflow, or if I have missed something so would appreciate your guidance.
SIDE QUESTION: Also, when I originally submitted this PR, I used a different Git config and user called "KillerKode" - however I was asked to change this, which is totally understandable so I changed everything to my name, however the commit history of that PR now contains some commits from my old Git config (as KillerKode) and some with my new Git config (as Faisal Mahmood). I am wondering, when this gets merged, how can I make sure that the commits are squashed and any history of my previous git config (i.e. KillerKode) do not show?
Kind regards,
Faisal.
On Mon, 15 Feb 2021 at 14:20, Faisal Mahmood <fasial.mahmood94@gmail.com> wrote:
Hello,
I hope you are all well, I currently have an issue / merge request that has become stale and as per the guidelines I am sending this e-mail to request someone to review it for me please.
Issue Number: 42861 (https://bugs.python.org/issue42861) Pull Request: 24180 (https://github.com/python/cpython/pull/24180)
This is my first contribution to CPython and I am very keen to get this added as I think it is an almost essential enhancement missing from the ipaddress module. I would greatly appreciate if someone can take a look and give me feedback, I am more than happy to help explain things, as I appreciate this is not the most straightforward thing to do.
Effectively, this method allows you to get the next closest network of any prefix size, it works by effectively adding 1 to the host portion of the network. There is actually a very nice guide someone made on stackexchange explaining how to do this calculation manually, I simply followed this logic and have tested it as much as possible and it seems to work great. See here: https://networkengineering.stackexchange.com/questions/7106/how-do-you-calcu...
Please let me know if there is anything else I can do to help get this merged.
Kind regards,
Faisal.
_______________________________________________ Python-Dev mailing list -- python-dev@python.org To unsubscribe send an email to python-dev-leave@python.org https://mail.python.org/mailman3/lists/python-dev.python.org/ Message archived at https://mail.python.org/archives/list/python-dev@python.org/message/DUL55S2S... Code of Conduct: http://python.org/psf/codeofconduct/
-- OpenPGP: https://hasan.d8u.us/openpgp.asc If you wish to request my time, please do so using *bit.ly/hd1AppointmentRequest <http://bit.ly/hd1AppointmentRequest>*. Si vous voudrais faire connnaisance, allez a *bit.ly/hd1AppointmentRequest <http://bit.ly/hd1AppointmentRequest>*. <https://sks-keyservers.net/pks/lookup?op=get&search=0xFEBAD7FFD041BBA1>Sent from my mobile device Envoye de mon portable
On Fri, 19 Mar 2021 at 17:26, Hasan Diwan <hasan.diwan@gmail.com> wrote:
For the side question, you can always use rebase instead of merge. Let me know if you need further particulars? -- H
There should be no need though, as the Python project has a policy of squash-merging all PRs. Paul
On Fri, Mar 19, 2021 at 9:29 AM Faisal Mahmood <fasial.mahmood94@gmail.com> wrote:
Hello,
Following my previous e-mail last month, thank you for responding. I almost immediately got two reviewers who posted helpful comments on my PR which I believe have all been resolved now.
Today, I got a notification to say my branch is stale again so wanted to understand what the next steps are.
Is there anything else I can do to get my PR merged? This is my first time submitting a PR to Cpython so not sure of the workflow, or if I have missed something so would appreciate your guidance.
At this point it's up to a core dev to have the time and inclination to look at the PR and either give feedback or to merge it (unfortunately I personally lack the knowledge to help out with this PR). -Brett
SIDE QUESTION: Also, when I originally submitted this PR, I used a different Git config and user called "KillerKode" - however I was asked to change this, which is totally understandable so I changed everything to my name, however the commit history of that PR now contains some commits from my old Git config (as KillerKode) and some with my new Git config (as Faisal Mahmood). I am wondering, when this gets merged, how can I make sure that the commits are squashed and any history of my previous git config (i.e. KillerKode) do not show?
Kind regards,
Faisal.
On Mon, 15 Feb 2021 at 14:20, Faisal Mahmood <fasial.mahmood94@gmail.com> wrote:
Hello,
I hope you are all well, I currently have an issue / merge request that has become stale and as per the guidelines I am sending this e-mail to request someone to review it for me please.
Issue Number: 42861 (https://bugs.python.org/issue42861) Pull Request: 24180 (https://github.com/python/cpython/pull/24180)
This is my first contribution to CPython and I am very keen to get this added as I think it is an almost essential enhancement missing from the ipaddress module. I would greatly appreciate if someone can take a look and give me feedback, I am more than happy to help explain things, as I appreciate this is not the most straightforward thing to do.
Effectively, this method allows you to get the next closest network of any prefix size, it works by effectively adding 1 to the host portion of the network. There is actually a very nice guide someone made on stackexchange explaining how to do this calculation manually, I simply followed this logic and have tested it as much as possible and it seems to work great. See here: https://networkengineering.stackexchange.com/questions/7106/how-do-you-calcu...
Please let me know if there is anything else I can do to help get this merged.
Kind regards,
Faisal.
_______________________________________________ Python-Dev mailing list -- python-dev@python.org To unsubscribe send an email to python-dev-leave@python.org https://mail.python.org/mailman3/lists/python-dev.python.org/ Message archived at https://mail.python.org/archives/list/python-dev@python.org/message/DUL55S2S... Code of Conduct: http://python.org/psf/codeofconduct/
On Mon, Feb 15, 2021 at 8:36 AM Faisal Mahmood <fasial.mahmood94@gmail.com> wrote:
Hello,
I hope you are all well, I currently have an issue / merge request that has become stale and as per the guidelines I am sending this e-mail to request someone to review it for me please.
Issue Number: 42861 (https://bugs.python.org/issue42861) Pull Request: 24180 (https://github.com/python/cpython/pull/24180)
Could you share some motivation references that could help explain why this next_network method will be helpful? Is this common enough to warrant a method in the stdlib IP-Address module? If there is prior-art in other external libraries or libraries of other languages, that will be helpful to know and review too. I had looked at this a month ago when you pinged for review, but I could not immediately determine the benefit of having this method in stdlib, irrespective of implementation correctness or API Signature. I also lack extensive experience with ipv6. Thanks, Senthil
Honestly this is an example of a module that would have been better off outside the stdlib. On Fri, Mar 19, 2021 at 14:43 Senthil Kumaran <senthil@python.org> wrote:
On Mon, Feb 15, 2021 at 8:36 AM Faisal Mahmood <fasial.mahmood94@gmail.com> wrote:
Hello,
I hope you are all well, I currently have an issue / merge request that
has become stale and as per the guidelines I am sending this e-mail to request someone to review it for me please.
Issue Number: 42861 (https://bugs.python.org/issue42861) Pull Request: 24180 (https://github.com/python/cpython/pull/24180)
Could you share some motivation references that could help explain why this next_network method will be helpful? Is this common enough to warrant a method in the stdlib IP-Address module? If there is prior-art in other external libraries or libraries of other languages, that will be helpful to know and review too.
I had looked at this a month ago when you pinged for review, but I could not immediately determine the benefit of having this method in stdlib, irrespective of implementation correctness or API Signature. I also lack extensive experience with ipv6.
Thanks, Senthil _______________________________________________ Python-Dev mailing list -- python-dev@python.org To unsubscribe send an email to python-dev-leave@python.org https://mail.python.org/mailman3/lists/python-dev.python.org/ Message archived at https://mail.python.org/archives/list/python-dev@python.org/message/OABKRDE2... Code of Conduct: http://python.org/psf/codeofconduct/
-- --Guido (mobile)
Thank you for taking the time to look at this. I do agree with Guido to an extent, the ipaddress module is by far the most comprehensive and useful standard IP library I've seen in a programming language, I was surprised when I first found it's existence. But being fairly new to Python, I assumed this was the Python way and it has been extremely useful to me as a Network Automation Engineer at my company. One way I have always seen Python as useful is that I don't have to keep reinventing the wheel or wasting time writing code that could have already been written in a standard way, I think it's one of the many important reasons why people choose Python, because the code for what they want to do probably already exists in stdlib. But whether this library should be in stdlib or not is beyond the scope of my PR, I think we are in the situation we are in and we should make sure the stdlib is maintained and kept up to date in the mean time. Here are the motivations for getting this added: FEELS INCOMPLETE: This library includes the subnet_of and supernet_of methods, they will effectively allow you to break down a network into smaller subnets (i.e. go down) or find the networks parent network (i.e. go up). But what they are missing is finding the next closest network (i.e. go sideways) - (which reminds me, perhaps I should implement a prev_network() as well?). It's always been a case where this library has felt incomplete as it lets you go in the up and down direction but omits the sideways direction. IT'S COMPLICATED: Finding the next closest network is not a trivial task to do in your head and it's difficult to understand, from my experience the most efficient way of doing it is on the binary level. When I first had a use case to do this, it took me a good amount of time trying to understand this algorithm and implement it, I'm certain there will be many people out there struggling with this and reinventing the wheel. By adding this into the ipaddress module, we help simplify this task and improve peoples network knowledge. A lot of users of the ipaddress module are likely to be Network Engineers who have little programming experience, they would probably not be experienced enough to implement something like this or get involved in feeding it back to the Python project. IT'S NEEDED: Personally, I almost exclusively work with IPv4 networks and one very common use case we get is slicing up networks to use them as efficiently as possible. For example, if I am given a network of 10.250.1.0/16 and I need to reserve a /21 for some servers within that range, I would initially just reserve at the start of the network, i.e. 10.250.1.0/21 - very simple. But now, say someone comes along and says they want a /19 from the same network, then it's not so simple anymore. The 10.250.1.0/21 network runs from 10.250.1.0-10.250.7.254 - you can't just take the broadcast address and add 1 to it (i.e. 10.250.8.0/19 would be invalid), the next closest /19 network is actually infact 10.250.32.0/19. There is no way to figure this out currently in the ipaddress module, even though it is a very common practice that is done by network engineers manually. A helpful way to illustrate this is by playing around on this VPC Subnet Builder here: https://tidalmigrations.com/subnet-builder/ - You can see a screenshot of my example below: [image: image.png] I hope this helps - please let me know if you need anything else. I would not have spent any time on this if I didn't think it was a valuable and fitting element of this module. To me it seemed like a very important omission, but that's just my two cents. On Sat, 20 Mar 2021 at 03:39, Guido van Rossum <guido@python.org> wrote:
Honestly this is an example of a module that would have been better off outside the stdlib.
On Fri, Mar 19, 2021 at 14:43 Senthil Kumaran <senthil@python.org> wrote:
On Mon, Feb 15, 2021 at 8:36 AM Faisal Mahmood <fasial.mahmood94@gmail.com> wrote:
Hello,
I hope you are all well, I currently have an issue / merge request that
has become stale and as per the guidelines I am sending this e-mail to request someone to review it for me please.
Issue Number: 42861 (https://bugs.python.org/issue42861) Pull Request: 24180 (https://github.com/python/cpython/pull/24180)
Could you share some motivation references that could help explain why this next_network method will be helpful? Is this common enough to warrant a method in the stdlib IP-Address module? If there is prior-art in other external libraries or libraries of other languages, that will be helpful to know and review too.
I had looked at this a month ago when you pinged for review, but I could not immediately determine the benefit of having this method in stdlib, irrespective of implementation correctness or API Signature. I also lack extensive experience with ipv6.
Thanks, Senthil _______________________________________________ Python-Dev mailing list -- python-dev@python.org To unsubscribe send an email to python-dev-leave@python.org https://mail.python.org/mailman3/lists/python-dev.python.org/ Message archived at https://mail.python.org/archives/list/python-dev@python.org/message/OABKRDE2... Code of Conduct: http://python.org/psf/codeofconduct/
-- --Guido (mobile)
On 20 Mar 2021, at 04:39, Guido van Rossum <guido@python.org> wrote:
Honestly this is an example of a module that would have been better off outside the stdlib.
On Fri, Mar 19, 2021 at 14:43 Senthil Kumaran <senthil@python.org> wrote: On Mon, Feb 15, 2021 at 8:36 AM Faisal Mahmood <fasial.mahmood94@gmail.com> wrote:
Hello,
I hope you are all well, I currently have an issue / merge request that has become stale and as per the guidelines I am sending this e-mail to request someone to review it for me please.
Issue Number: 42861 (https://bugs.python.org/issue42861) Pull Request: 24180 (https://github.com/python/cpython/pull/24180)
Could you share some motivation references that could help explain why this next_network method will be helpful? Is this common enough to warrant a method in the stdlib IP-Address module? If there is prior-art in other external libraries or libraries of other languages, that will be helpful to know and review too.
I had looked at this a month ago when you pinged for review, but I could not immediately determine the benefit of having this method in stdlib, irrespective of implementation correctness or API Signature. I also lack extensive experience with ipv6.
Thanks, Senthil
I don’t know if this is gonna support Faisal’s cause (“It’s used in a third party library in the wild, that means stdlib could use it!”) or the exact opposite (“it’s provided by a third party library, so may as well use third party library and stdlib doesn’t necessarily need this”) but the quite popular netaddr library that I’m a maintainer of does have IPNetwork.next() and IPNetwork.previous() methods[1][2]. The main difference is that in netaddr: * next() and previous() can produce networks arbitrary number of steps away, not just the first closest network forwards or backwards * going from a network to its supernetwork or subnetwork is done through a separate set of supernet() and subnet() methods[3][4] I can’t say how many library users actually consume this particular section of the API, of course, but I expect this API to be used and it seems rather useful. Best, Jakub [1] https://netaddr.readthedocs.io/en/latest/api.html#netaddr.IPNetwork.next [2] https://netaddr.readthedocs.io/en/latest/api.html#netaddr.IPNetwork.previous [3] https://netaddr.readthedocs.io/en/latest/api.html#netaddr.IPNetwork.subnet [4] https://netaddr.readthedocs.io/en/latest/api.html#netaddr.IPNetwork.supernet
Thanks Jakub, I wasn't aware of the netaddr library but just gave it a play and it does seem very similar and I think it's very useful and completely valid. I think the subtle difference is also that my implementation allows you to specify the next prefix as well, so it won't just find the next prefix of the same size. This is a common use case when you are trying to find free address spaces, you will need to look for networks of different sizes depending on what you are doing. Currently, you could say that you were given a network of 10.200.20.0/24 and asked to split this network into a bunch of /26 networks, you can do this easily using the subnets method: *>>> list(IPv4Network("10.200.20.0/24 <http://10.200.20.0/24>").subnets(prefixlen_diff=2))* *[IPv4Network('10.200.20.0/26' <http://10.200.20.0/26'>), IPv4Network('10.200.20.64/26' <http://10.200.20.64/26'>), IPv4Network('10.200.20.128/26' <http://10.200.20.128/26'>), IPv4Network('10.200.20.192/26' <http://10.200.20.192/26'>)]* That is very simple and effective, but not a very realistic example of how you would split networks up. Given how limited the IPv4 address space is, normally you may have to use that /24 block for multiple things, so can't just simply split it up into /26's, you may need to instead get two /30's, one /27 and one /25. Currently, I don't think there is any straightforward way to do this without a 'next_network' method that I have implemented. Example, given a network of 10.200.20.0/24, to get two /30's out of it, one /27 and one /25, I would do the following with my method: *>>> first_network = IPv4Network("10.200.20.0/30 <http://10.200.20.0/30>")* *# first_network = IPv4Network("10.200.20.0/30 <http://10.200.20.0/30>")* Then get the next one (note not specifying prefix just gives me another /30 - i.e. same prefix size): *>>> second_network = first_network.next_network()* *# second_network = IPv4Network("10.200.20.4/30 <http://10.200.20.4/30>")* Then I would need to get the /27, so do this: *>>> third_network = second_network.next_network(new_prefixlen=27)* *# third_network = IPv4Network("10.200.20.32/27 <http://10.200.20.32/27>)* Finally the /25: *>>> fourth_network = third_network.next_network(new_prefixlen=25)* *# fourth_network = IPv4Network("10.200.20.128/25 <http://10.200.20.128/25>)* When you are dealing with the same prefix size for each new network, I think it's just a simple case of adding 1 to the broadcast address each time, but when you have different prefix sizes it's a bit more complicated. On Sat, 20 Mar 2021 at 22:04, Jakub Stasiak <jakub@stasiak.at> wrote:
On 20 Mar 2021, at 04:39, Guido van Rossum <guido@python.org> wrote:
Honestly this is an example of a module that would have been better off outside the stdlib.
On Fri, Mar 19, 2021 at 14:43 Senthil Kumaran <senthil@python.org> wrote: On Mon, Feb 15, 2021 at 8:36 AM Faisal Mahmood <fasial.mahmood94@gmail.com> wrote:
Hello,
I hope you are all well, I currently have an issue / merge request
that has become stale and as per the guidelines I am sending this e-mail to request someone to review it for me please.
Issue Number: 42861 (https://bugs.python.org/issue42861) Pull Request: 24180 (https://github.com/python/cpython/pull/24180)
Could you share some motivation references that could help explain why this next_network method will be helpful? Is this common enough to warrant a method in the stdlib IP-Address module? If there is prior-art in other external libraries or libraries of other languages, that will be helpful to know and review too.
I had looked at this a month ago when you pinged for review, but I could not immediately determine the benefit of having this method in stdlib, irrespective of implementation correctness or API Signature. I also lack extensive experience with ipv6.
Thanks, Senthil
I don’t know if this is gonna support Faisal’s cause (“It’s used in a third party library in the wild, that means stdlib could use it!”) or the exact opposite (“it’s provided by a third party library, so may as well use third party library and stdlib doesn’t necessarily need this”) but the quite popular netaddr library that I’m a maintainer of does have IPNetwork.next() and IPNetwork.previous() methods[1][2]. The main difference is that in netaddr:
* next() and previous() can produce networks arbitrary number of steps away, not just the first closest network forwards or backwards * going from a network to its supernetwork or subnetwork is done through a separate set of supernet() and subnet() methods[3][4]
I can’t say how many library users actually consume this particular section of the API, of course, but I expect this API to be used and it seems rather useful.
Best, Jakub
[1] https://netaddr.readthedocs.io/en/latest/api.html#netaddr.IPNetwork.next [2] https://netaddr.readthedocs.io/en/latest/api.html#netaddr.IPNetwork.previous [3] https://netaddr.readthedocs.io/en/latest/api.html#netaddr.IPNetwork.subnet [4] https://netaddr.readthedocs.io/en/latest/api.html#netaddr.IPNetwork.supernet
On 22 Mar 2021, at 13:07, Faisal Mahmood <fasial.mahmood94@gmail.com> wrote:
Thanks Jakub, I wasn't aware of the netaddr library but just gave it a play and it does seem very similar and I think it's very useful and completely valid.
I think the subtle difference is also that my implementation allows you to specify the next prefix as well, so it won't just find the next prefix of the same size. This is a common use case when you are trying to find free address spaces, you will need to look for networks of different sizes depending on what you are doing.
Currently, you could say that you were given a network of 10.200.20.0/24 and asked to split this network into a bunch of /26 networks, you can do this easily using the subnets method:
list(IPv4Network("10.200.20.0/24").subnets(prefixlen_diff=2)) [IPv4Network('10.200.20.0/26'), IPv4Network('10.200.20.64/26'), IPv4Network('10.200.20.128/26'), IPv4Network('10.200.20.192/26')]
That is very simple and effective, but not a very realistic example of how you would split networks up. Given how limited the IPv4 address space is, normally you may have to use that /24 block for multiple things, so can't just simply split it up into /26's, you may need to instead get two /30's, one /27 and one /25. Currently, I don't think there is any straightforward way to do this without a 'next_network' method that I have implemented.
Example, given a network of 10.200.20.0/24, to get two /30's out of it, one /27 and one /25, I would do the following with my method:
first_network = IPv4Network("10.200.20.0/30") # first_network = IPv4Network("10.200.20.0/30")
Then get the next one (note not specifying prefix just gives me another /30 - i.e. same prefix size):
second_network = first_network.next_network() # second_network = IPv4Network("10.200.20.4/30")
Then I would need to get the /27, so do this:
third_network = second_network.next_network(new_prefixlen=27) # third_network = IPv4Network("10.200.20.32/27)
Finally the /25:
fourth_network = third_network.next_network(new_prefixlen=25) # fourth_network = IPv4Network("10.200.20.128/25)
When you are dealing with the same prefix size for each new network, I think it's just a simple case of adding 1 to the broadcast address each time, but when you have different prefix sizes it's a bit more complicated.
On Sat, 20 Mar 2021 at 22:04, Jakub Stasiak <jakub@stasiak.at> wrote:
I don’t know if this is gonna support Faisal’s cause (“It’s used in a third party library in the wild, that means stdlib could use it!”) or the exact opposite (“it’s provided by a third party library, so may as well use third party library and stdlib doesn’t necessarily need this”) but the quite popular netaddr library that I’m a maintainer of does have IPNetwork.next() and IPNetwork.previous() methods[1][2]. The main difference is that in netaddr:
* next() and previous() can produce networks arbitrary number of steps away, not just the first closest network forwards or backwards * going from a network to its supernetwork or subnetwork is done through a separate set of supernet() and subnet() methods[3][4]
I can’t say how many library users actually consume this particular section of the API, of course, but I expect this API to be used and it seems rather useful.
Best, Jakub
[1] https://netaddr.readthedocs.io/en/latest/api.html#netaddr.IPNetwork.next [2] https://netaddr.readthedocs.io/en/latest/api.html#netaddr.IPNetwork.previous [3] https://netaddr.readthedocs.io/en/latest/api.html#netaddr.IPNetwork.subnet [4] https://netaddr.readthedocs.io/en/latest/api.html#netaddr.IPNetwork.supernet
Thank you for the explanation – now I understand better how that prefix parameter is useful. It’s not *that* difficult to emulate this using netaddr, there’s an extra switch-to-supernet or switch-to-subnet step but that’s about it:
from netaddr import IPNetwork first_network = IPNetwork("10.200.20.0/30") second_network = first_network.next() second_network IPNetwork('10.200.20.4/30') third_network = second_network.supernet(27)[0].next() third_network IPNetwork('10.200.20.32/27') fourth_network = third_network.supernet(25)[0].next() fourth_network IPNetwork('10.200.20.128/25’)
That said, if merging this into the Python stdlib doesn’t work out I’m open to improving netaddr’s interface to better serve this use case. Best wishes, Jakub
Given this discussion, I am motivated to review this and merge this into stdlib. The conversation was helpful to me to understand for utility value of the method introduced by the patch. I had stated in the PR as well. Thank you, both, Faisal and Jakub. On Mon, Mar 22, 2021 at 5:40 PM Jakub Stasiak <jakub@stasiak.at> wrote:
On 22 Mar 2021, at 13:07, Faisal Mahmood <fasial.mahmood94@gmail.com> wrote:
Thanks Jakub, I wasn't aware of the netaddr library but just gave it a play and it does seem very similar and I think it's very useful and completely valid.
I think the subtle difference is also that my implementation allows you to specify the next prefix as well, so it won't just find the next prefix of the same size. This is a common use case when you are trying to find free address spaces, you will need to look for networks of different sizes depending on what you are doing.
Currently, you could say that you were given a network of 10.200.20.0/24 and asked to split this network into a bunch of /26 networks, you can do this easily using the subnets method:
list(IPv4Network("10.200.20.0/24").subnets(prefixlen_diff=2)) [IPv4Network('10.200.20.0/26'), IPv4Network('10.200.20.64/26'), IPv4Network('10.200.20.128/26'), IPv4Network('10.200.20.192/26')]
That is very simple and effective, but not a very realistic example of how you would split networks up. Given how limited the IPv4 address space is, normally you may have to use that /24 block for multiple things, so can't just simply split it up into /26's, you may need to instead get two /30's, one /27 and one /25. Currently, I don't think there is any straightforward way to do this without a 'next_network' method that I have implemented.
Example, given a network of 10.200.20.0/24, to get two /30's out of it, one /27 and one /25, I would do the following with my method:
first_network = IPv4Network("10.200.20.0/30") # first_network = IPv4Network("10.200.20.0/30")
Then get the next one (note not specifying prefix just gives me another /30 - i.e. same prefix size):
second_network = first_network.next_network() # second_network = IPv4Network("10.200.20.4/30")
Then I would need to get the /27, so do this:
third_network = second_network.next_network(new_prefixlen=27) # third_network = IPv4Network("10.200.20.32/27)
Finally the /25:
fourth_network = third_network.next_network(new_prefixlen=25) # fourth_network = IPv4Network("10.200.20.128/25)
When you are dealing with the same prefix size for each new network, I think it's just a simple case of adding 1 to the broadcast address each time, but when you have different prefix sizes it's a bit more complicated.
On Sat, 20 Mar 2021 at 22:04, Jakub Stasiak <jakub@stasiak.at> wrote:
I don’t know if this is gonna support Faisal’s cause (“It’s used in a third party library in the wild, that means stdlib could use it!”) or the exact opposite (“it’s provided by a third party library, so may as well use third party library and stdlib doesn’t necessarily need this”) but the quite popular netaddr library that I’m a maintainer of does have IPNetwork.next() and IPNetwork.previous() methods[1][2]. The main difference is that in netaddr:
* next() and previous() can produce networks arbitrary number of steps away, not just the first closest network forwards or backwards * going from a network to its supernetwork or subnetwork is done through a separate set of supernet() and subnet() methods[3][4]
I can’t say how many library users actually consume this particular section of the API, of course, but I expect this API to be used and it seems rather useful.
Best, Jakub
[1] https://netaddr.readthedocs.io/en/latest/api.html#netaddr.IPNetwork.next [2] https://netaddr.readthedocs.io/en/latest/api.html#netaddr.IPNetwork.previous [3] https://netaddr.readthedocs.io/en/latest/api.html#netaddr.IPNetwork.subnet [4] https://netaddr.readthedocs.io/en/latest/api.html#netaddr.IPNetwork.supernet
Thank you for the explanation – now I understand better how that prefix parameter is useful.
It’s not *that* difficult to emulate this using netaddr, there’s an extra switch-to-supernet or switch-to-subnet step but that’s about it:
from netaddr import IPNetwork first_network = IPNetwork("10.200.20.0/30") second_network = first_network.next() second_network IPNetwork('10.200.20.4/30') third_network = second_network.supernet(27)[0].next() third_network IPNetwork('10.200.20.32/27') fourth_network = third_network.supernet(25)[0].next() fourth_network IPNetwork('10.200.20.128/25’)
That said, if merging this into the Python stdlib doesn’t work out I’m open to improving netaddr’s interface to better serve this use case.
Best wishes, Jakub
On 3/22/2021 8:51 PM, Senthil Kumaran wrote:
Given this discussion, I am motivated to review this and merge this into stdlib. The conversation was helpful to me to understand for utility value of the method introduced by the patch. I had stated in the PR as well.
Thank you, both, Faisal and Jakub.
Yes, thank you. I've also needed the "next network of a given prefix size" feature. Eric
On Mon, Mar 22, 2021 at 5:40 PM Jakub Stasiak <jakub@stasiak.at> wrote:
On 22 Mar 2021, at 13:07, Faisal Mahmood <fasial.mahmood94@gmail.com> wrote:
Thanks Jakub, I wasn't aware of the netaddr library but just gave it a play and it does seem very similar and I think it's very useful and completely valid.
I think the subtle difference is also that my implementation allows you to specify the next prefix as well, so it won't just find the next prefix of the same size. This is a common use case when you are trying to find free address spaces, you will need to look for networks of different sizes depending on what you are doing.
Currently, you could say that you were given a network of 10.200.20.0/24 and asked to split this network into a bunch of /26 networks, you can do this easily using the subnets method:
list(IPv4Network("10.200.20.0/24").subnets(prefixlen_diff=2)) [IPv4Network('10.200.20.0/26'), IPv4Network('10.200.20.64/26'), IPv4Network('10.200.20.128/26'), IPv4Network('10.200.20.192/26')]
That is very simple and effective, but not a very realistic example of how you would split networks up. Given how limited the IPv4 address space is, normally you may have to use that /24 block for multiple things, so can't just simply split it up into /26's, you may need to instead get two /30's, one /27 and one /25. Currently, I don't think there is any straightforward way to do this without a 'next_network' method that I have implemented.
Example, given a network of 10.200.20.0/24, to get two /30's out of it, one /27 and one /25, I would do the following with my method:
first_network = IPv4Network("10.200.20.0/30") # first_network = IPv4Network("10.200.20.0/30")
Then get the next one (note not specifying prefix just gives me another /30 - i.e. same prefix size):
second_network = first_network.next_network() # second_network = IPv4Network("10.200.20.4/30")
Then I would need to get the /27, so do this:
third_network = second_network.next_network(new_prefixlen=27) # third_network = IPv4Network("10.200.20.32/27)
Finally the /25:
fourth_network = third_network.next_network(new_prefixlen=25) # fourth_network = IPv4Network("10.200.20.128/25)
When you are dealing with the same prefix size for each new network, I think it's just a simple case of adding 1 to the broadcast address each time, but when you have different prefix sizes it's a bit more complicated.
On Sat, 20 Mar 2021 at 22:04, Jakub Stasiak <jakub@stasiak.at> wrote:
I don’t know if this is gonna support Faisal’s cause (“It’s used in a third party library in the wild, that means stdlib could use it!”) or the exact opposite (“it’s provided by a third party library, so may as well use third party library and stdlib doesn’t necessarily need this”) but the quite popular netaddr library that I’m a maintainer of does have IPNetwork.next() and IPNetwork.previous() methods[1][2]. The main difference is that in netaddr:
* next() and previous() can produce networks arbitrary number of steps away, not just the first closest network forwards or backwards * going from a network to its supernetwork or subnetwork is done through a separate set of supernet() and subnet() methods[3][4]
I can’t say how many library users actually consume this particular section of the API, of course, but I expect this API to be used and it seems rather useful.
Best, Jakub
[1] https://netaddr.readthedocs.io/en/latest/api.html#netaddr.IPNetwork.next [2] https://netaddr.readthedocs.io/en/latest/api.html#netaddr.IPNetwork.previous [3] https://netaddr.readthedocs.io/en/latest/api.html#netaddr.IPNetwork.subnet [4] https://netaddr.readthedocs.io/en/latest/api.html#netaddr.IPNetwork.supernet Thank you for the explanation – now I understand better how that prefix parameter is useful.
It’s not *that* difficult to emulate this using netaddr, there’s an extra switch-to-supernet or switch-to-subnet step but that’s about it:
from netaddr import IPNetwork first_network = IPNetwork("10.200.20.0/30") second_network = first_network.next() second_network IPNetwork('10.200.20.4/30') third_network = second_network.supernet(27)[0].next() third_network IPNetwork('10.200.20.32/27') fourth_network = third_network.supernet(25)[0].next() fourth_network IPNetwork('10.200.20.128/25’)
That said, if merging this into the Python stdlib doesn’t work out I’m open to improving netaddr’s interface to better serve this use case.
Best wishes, Jakub
_______________________________________________ Python-Dev mailing list -- python-dev@python.org To unsubscribe send an email to python-dev-leave@python.org https://mail.python.org/mailman3/lists/python-dev.python.org/ Message archived at https://mail.python.org/archives/list/python-dev@python.org/message/LPCY3R4W... Code of Conduct: http://python.org/psf/codeofconduct/
-- Eric V. Smith
participants (8)
-
Brett Cannon
-
Eric V. Smith
-
Faisal Mahmood
-
Guido van Rossum
-
Hasan Diwan
-
Jakub Stasiak
-
Paul Moore
-
Senthil Kumaran