-
Notifications
You must be signed in to change notification settings - Fork 7
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
feature: Added support to list[list[int]]
input
#428
Conversation
- Moved get_tag_ranges()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's shaping up well all things considered that have been discussed on the issue discussion too. I've done a first review pass, I'll do another one in the future.
@Alopalao I'll also wait for a future PR with mef_eline
using this PR, just so it's also easier to test in practice with a NApp using it for real (as you're already doing locally that you mentioned). At that time you can also ship the new e2e tests covering how the uni vlan range will be used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great PR, @Alopalao, I've just done a second pass, and it was great to also explore it a bit with mef_eline
PR. I've raised a few minor points that we need to sort out. Finally, let's update the changelog too after these open threads are addressed.
Let's also make sure e2e are passing as you mentioned, it'd be great to also have the new mef_eline vlan ranges e2e too.
Final observation, I haven't stress tested it, as you've done it, but regarding the low digit ms worst case scenarios let's also explore it again and see if locks and anything else might get impacted.
- Updated docstrings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nicely done, @Alopalao, this is pretty much ready to land. I've opened a few minor threads though. Also, changelog hasn't been updated.
- Updated docstrings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Alopalao appreciated your updates, it's pretty much ready to land, if you could address the last thread that I opened and also update the changelog, after that I'll approve this PR. This is a fantastic contribution unlocking vlan range usage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Closes #427
Summary
Added
range_addition()
. Its presence is not final but is used in the meantime.Added exceptions
KytosInvalidRanges
,KytosTagsAreNotAvailable
andKytosTagsNotInTagRanges
Added support for input
list[list[int]]
touse_tags()
andmake_tags_available()
These methods also have changed their return. For
use_tags()
there is no longer a return value, instead, it raisesKytosTagsAreNotAvailable
for any errors.make_tags_available()
returns any conflicting values, this does not stop the addition process. It can also raiseKytosTagsNotInTagRanges
.Created the file
tag_ranges.py
that contains every Interface range-related method that was static.Created
TAGRange
class for tags that have a list of ranges of tags.Moved
get_tag_ranges()
andmap_singular_values()
totag_ranges.py
so it can be used for other, currentlymef_eline
andtopology
are planned to use it.Local Tests
vlan_range
developmentEnd-To-End Tests