-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Add CREATED column to podman artifact ls output #27325
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
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: rhatdan The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore. |
Did we decide that these commands are stable? This is technically a CLI interface break. |
Since I am probably the first one to use these interfaces, I would say no, it was not stable in the past, I have found too many issues. Adding more information to the output, does not seem like a real break to me. Not having this information in the output is what I would consider a bug. |
I'm not against adding the column but I do object to you finding |
cmd/podman/artifact/list.go
Outdated
} | ||
|
||
// Get the created time from the manifest annotations | ||
created := "<unknown>" |
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.
i think created should be blank if nothing is found.
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.
just grepped around and I dont think we do this with other objects.
created := "<unknown>" | ||
if lr.Artifact.Manifest != nil && lr.Artifact.Manifest.Annotations != nil { | ||
if createdStr, ok := lr.Artifact.Manifest.Annotations[imgspec.AnnotationCreated]; ok { | ||
if createdTime, err := time.Parse(time.RFC3339Nano, createdStr); err == nil { |
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.
should we logrus if we find "bad" input?
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.
I don't think so, I am fine with moving to "" for created time.
This change adds a CREATED column to the podman artifact ls command output, displaying the artifact creation time in human-readable format (e.g., "3 weeks ago"), similar to podman image ls. The creation time is extracted from the org.opencontainers.image.created annotation stored in the artifact manifest and formatted using units.HumanDuration. Changes: - Add Created field to artifactListOutput struct - Parse and format creation time from manifest annotations - Update default output format to include CREATED column - Update documentation with Created placeholder - Add test to verify Created column is displayed Fixes containers#27314 Co-authored-by: Cursor AI <[email protected]> Signed-off-by: Daniel J Walsh <[email protected]>
Do not take offense on reporting issues, but:
While tools using podman artifact can work around a lot of these, it makes it a lot easier to handle when the functionality matches other commands. |
Closing in favor of #27319 |
This change adds a CREATED column to the podman artifact ls command output, displaying the artifact creation time in human-readable format (e.g., "3 weeks ago"), similar to podman image ls.
The creation time is extracted from the org.opencontainers.image.created annotation stored in the artifact manifest and formatted using units.HumanDuration.
Changes:
Fixes #27314
Does this PR introduce a user-facing change?