Handle output from call_cmd better in maas_storage #283
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
plars
force-pushed
the
maas-call-cmd-improved
branch
from
0961e33
to
67830ab
Compare
# for free
to join this conversation on GitHub.
Already have an account?
# 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.
Description
I was looking at some other things with the maas device connector and recalled that there were some gaps in how output is handled from this call_cmd method in maas_storage. There are some failure modes that wouldn't produce the output you would expect, and some places where it should raise an exception instead. This should close those gaps.
I also noticed while testing this that the previous unit tests fail on python3.12 because there's a way it can get TypeError when trying to run the command with None in it. This handles that potential error better, and also adds a way to inject the node_info so that testing is easier
Resolved issues
Resolves #264
Documentation
N/A
Web service API changes
N/A
Tests
Lots of test improvements, including a way to inject node_info in MaasStorage.
+4% coverage of maas_storage with the additional unit tests added too