Conversation
|
What should I do to not decrease the coverage? Not sure what the problem is :) |
|
@sbueringer The coverage can be ignored - sometimes it calculates things oddly. However, we do need to see the actual Nova python code where this feature is implemented. See here for more details and let me know if you have any questions. |
|
@jtopjian Okay thx. I added the link to the code where the os volume attachments are added above |
|
Build succeeded.
|
jtopjian
left a comment
There was a problem hiding this comment.
@sbueringer Thanks for adding the code reference.
I've left a comment about an alternative way of implementing this. Please let me know if you have any questions.
| SecurityGroups []map[string]interface{} `json:"security_groups"` | ||
|
|
||
| // AttachedVolumes includes the volume attachments of this instance | ||
| AttachedVolumes []map[string]string `json:"os-extended-volumes:volumes_attached"` |
There was a problem hiding this comment.
Instead of []map[string]string, how about:
type AttachedVolume struct {
ID string `json:"id"`
}and then
// AttachedVolumes includes the volume attachments of this instance
AttachedVolumes []AttachedVolume `json:"os-extended-volumes:volumes_attached"`According to the code, id is the only field except if microversion 2.3+ is used, and then there is a delete_on_termination field. But since that's a microservice addition, we'll have to handle that differently, so let's not worry about that in this PR.
There was a problem hiding this comment.
Sounds good to me I pushed a fix.
|
Build succeeded.
|
|
@jtopjian PTAL :) |
|
Thx for the fast review ;) |
For #1731
Links to the line numbers/files in the OpenStack source code that support the
code in this PR: