-
Notifications
You must be signed in to change notification settings - Fork 2
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
Subnet routes refactoring #205
base: stable/yoga-m3
Are you sure you want to change the base?
Conversation
route_on_active implies that this option is for turning on and off provisioning of routes to active devices. It's the opposite however - turning on and off provisioning to passive devices.
all(... for x in []) yields True, which is desired here, but can still be unexpected behavior to a reader. An explicit check for [] helps readability without changing behavior.
- Clearer variable names - Sections with section header comments - More efficient decision logic So far we're iterating over all routes and `continue` when we want to skip a route. But multiple checks require iterating over all networks and/or subnets. - Every check has to iterate over networks separately - `continue` can't be used to ignore routes unless we iterate over networks using list comprehension So put decision logic into a function ignore_route. In it we iterate over networks once. If a route is to be ignored (i. e. spared from deletion) we can return immediately, no matter how deeply nested the current for-loop is. - Fix formatting
6bfc3de
to
1ae285e
Compare
Since subnet deletion was separated out into the CleanupSubnetRoutes function in c81eb89 it makes sense to keep the names more in line with the naming convention used for other tasks that separate between creation and deletion
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.
Rename SyncSubnetRoutes to EnsureSubnetRoutes
<- the name change is not justified since the subnet routes are still removed by the same function - and in fact the same function could also cleanup by providing it an empty selfip array which makes the new "cleanup" function redundant.
Currently waiting for fix of member/SelfIP conflict (which already includes some heavy L2 refactoring)
This PR follows up on #204.
So far it stacks onto (i.e. includes) #204, but I will rebase it onto
stable/yoga-m3
once #204 is merged.