Fix TypeError on missing network in update_port_postcommit() #126
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When a port is updated we try to clean up after it. As there is no explicit delete message, but only a update_port_postcommit() call, we handle deletion of the old host. For this, the delete call also needs the network, but in certain scenarios, there is no original network - most likely in cases where the port only gets its binding host updated, but stays in the same network. Therefore in these cases we now use the network_id given in the original bindings level. As segments are not always fully specified (or better: this is what I have seen in some places in the code, but am unsure if the segments are always fully specified or not within a NetworkContext (fully specified in this case means "missing the network_id entry")), I chose the "safe way", using the original network first, then look at the segment dict and if that fails we log a warning and return, i.e. don't try to clean up after the segment. This fixes a TypeError, where when there was no original network, we'd try to lookup ['id'] to the original network, which was None.
swagner-de
approved these changes
Sep 27, 2023
sebageek
deleted the
fix-update-port-postcommit-with-no-orig-network
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When a port is updated we try to clean up after it. As there is no explicit delete message, but only a update_port_postcommit() call, we handle deletion of the old host. For this, the delete call also needs the network, but in certain scenarios, there is no original network - most likely in cases where the port only gets its binding host updated, but stays in the same network. Therefore in these cases we now use the network_id given in the original bindings level. As segments are not always fully specified (or better: this is what I have seen in some places in the code, but am unsure if the segments are always fully specified or not within a NetworkContext (fully specified in this case means "missing the network_id entry")), I chose the "safe way", using the original network first, then look at the segment dict and if that fails we log a warning and return, i.e. don't try to clean up after the segment.
This fixes a TypeError, where when there was no original network, we'd try to lookup ['id'] to the original network, which was None.