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

Added feature to avoid tags when selecting s_vlan #517

Merged
merged 4 commits into from
Nov 25, 2024

Conversation

Alopalao
Copy link

Closes #516

Summary

Added argument to accept a TAG to be avoided.

Local Tests

Tested with calling link.get_next_available_tag(controller, link.id, avoid_tag=tag) on Kytos console.
Added unit test to cover all possible scenarios.

End-to-End Tests

N/A

@Alopalao Alopalao requested a review from a team as a code owner November 19, 2024 21:38
Copy link
Member

@viniarck viniarck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Left a minor naming suggestion and docstring.

Other than that, this change also deserves to be in the changelog since it's a new sub-feature that's being exposed allowing NApps to try to avoid a given tag value when getting the next link tag.

kytos/core/link.py Outdated Show resolved Hide resolved
tests/unit/test_core/test_link.py Show resolved Hide resolved
- Change parameter avoid_tag: TAG to try_avoid_value: int.
Copy link
Member

@viniarck viniarck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely done @Alopalao, before merging, in the changelog, if you can still keep the information about the last tag feature that'd be great. Looks like you meant to only augment that line, but it deleted prior important information.

@viniarck viniarck merged commit dda6ebb into master Nov 25, 2024
1 of 2 checks passed
@viniarck viniarck deleted the feature/avoid_tags branch November 25, 2024 12:34
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 this pull request may close these issues.

Avoid Link s_vlan duplication
2 participants