-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Show volume options in 'volume inspect' #4284
Show volume options in 'volume inspect' #4284
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mheon The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
LGTM |
@mheon Should probably add a tests to make sure we don't regress in the future. |
Added a test |
72ed60c
to
bf07ddb
Compare
We initialized the map to show them, but didn't actually copy them in, so they weren't being displayed. Signed-off-by: Matthew Heon <[email protected]>
bf07ddb
to
6456f6d
Compare
Well that doesn't look right. I think |
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.
Code LGTM but the remote tests aren't happy
I have rewritten the remote endpoint for |
We need to use the new Inspect() endpoint instead of trying to JSON the actual volume structs. Currently, the output seems completely nonsensical; it seems like we're JSONing the struct for the Varlink connection itself? This should restore sanity and match the format of remote and local inspect on volumes. Signed-off-by: Matthew Heon <[email protected]>
73359e6
to
03da8b6
Compare
Tests passing now |
/lgtm |
We initialized the map to show them, but didn't actually copy them in, so they weren't being displayed.