Do not deprecate vm_nic records return value #206
Conversation
| type: list | ||
| elements: str | ||
| sample: 192.0.2.1 | ||
| records: |
There was a problem hiding this comment.
Here the "type" needs to be list? Also the second line under "description" is not needed anymore
There was a problem hiding this comment.
List is ok, a single NIC can have multiple IP addresses.
There was a problem hiding this comment.
Hm, I can see now that it is not clear that this question is meant for "records"
There was a problem hiding this comment.
Removed the second line from "description", thank you for noticing.
There was a problem hiding this comment.
And type: list, elements: dict.
| assert success is True | ||
| assert results == { | ||
| "changed": False, | ||
| "records": {}, |
There was a problem hiding this comment.
Should there be an empty list for the "records"? In the case of ensure_absent() an empty list is returned
There was a problem hiding this comment.
I guess we have incorrectly mocked return values. I do not want to extend this PR with fixing unittests.
justinc1
force-pushed
the
fix-vm-nic-records
branch
from
81110c5 to
60f329e
Compare
The module does work with list of NICs (the items input parameter). It is natural to also return a list of NICs. Reverts db9dff7 Signed-off-by: Justin Cinkelj <justin.cinkelj@xlab.si>
justinc1
force-pushed
the
fix-vm-nic-records
branch
from
60f329e to
23f7e29
Compare
The module does work with list of NICs (the items input parameter). It is natural to also return a list of NICs.
Reverts db9dff7
Draft: until at least one more person agrees for vm_nic module
recordsis ok, andrecordis not.