many: send components information on GET /v2/snaps #14060
Conversation
github-actions
bot
added
the
Needs Documentation -auto-
alfonsosanchezbeato
force-pushed
the
comp-api-send-components-info
branch
2 times, most recently
from
ea3afc4
to
5b2a83e
Compare
| // when its symlink was created. We cannot use the mount directory as lstat | ||
| // returns the date of the root of the container instead of the date when the | ||
| // mount directory was created. | ||
| func ComponentInstallDate(cpi ContainerPlaceInfo, snapRev Revision) *time.Time { |
There was a problem hiding this comment.
Should this follow the same pattern as snap.InstallDate? It returns a time.Time{} when we can't figure out the time, rather than a nil pointer.
There was a problem hiding this comment.
Although I see that you're using a pointer inside of client.Component, so it works well with omitempty. So this is probably fine.
There was a problem hiding this comment.
But Info.InstallDate() returns *time.Time so we are already inconsistent. I think that omitempty would work fine also if we return 0 though.
snap/component.go
Outdated
| } | ||
|
|
||
| // ComponentSize returns the file size of a component. | ||
| func ComponentSize(cpi ContainerPlaceInfo) int64 { |
There was a problem hiding this comment.
This also feels like it should return an error here if something goes wrong, and make the caller decide if it wants to default to zero?
There was a problem hiding this comment.
@andrewphelpsj thanks for your review, I've addressed your comments now
| // when its symlink was created. We cannot use the mount directory as lstat | ||
| // returns the date of the root of the container instead of the date when the | ||
| // mount directory was created. | ||
| func ComponentInstallDate(cpi ContainerPlaceInfo, snapRev Revision) *time.Time { |
There was a problem hiding this comment.
But Info.InstallDate() returns *time.Time so we are already inconsistent. I think that omitempty would work fine also if we return 0 though.
snap/component.go
Outdated
| } | ||
|
|
||
| // ComponentSize returns the file size of a component. | ||
| func ComponentSize(cpi ContainerPlaceInfo) int64 { |
There was a problem hiding this comment.
changes look good in themselves, but I think we need to think how to expose also the information about defined but not installed components, the spec requires to print #installed/#defined iirc. We might have to extend select to take a comma separated list of selectors or introduce select-components?
client/packages.go
Outdated
| @@ -91,6 +91,8 @@ type Snap struct { | |||
| GatingHold *time.Time `json:"gating-hold,omitempty"` | |||
| // if RefreshInhibit is nil, then there is no pending refresh. | |||
| RefreshInhibit *SnapRefreshInhibit `json:"refresh-inhibit,omitempty"` | |||
|
|
|||
| Comps []Component `json:"components,omitempty"` | |||
There was a problem hiding this comment.
I would call this Components, we don't seem to use .Comps anywhere else
That is a good point. I think that an reasonable solution would be that the list of returned components in a GET could also include non-installed components, and we can easily differentiate between both cases as the non-installed won't have `install-date/installed-size" fields. |
|
@pedronis I've changed this now so the not installed components are sent as well. |
alfonsosanchezbeato
force-pushed
the
comp-api-send-components-info
branch
from
a0d59fd
to
2c3bc45
Compare
|
Commits squashed now |
Send back components information on GET /v2/snaps and GET /v2/snaps/{name} requests.