-
Notifications
You must be signed in to change notification settings - Fork 1
feat: Add invocation history command and per-invocation log streaming #17
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
8a28557 to
f656eb1
Compare
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.
Performed full review of ff52a92...f656eb1
Analysis
-
The PR switches from
ListAutoPagingtoListfor API calls, which may create limitations when dealing with large datasets since pagination handling has been removed. -
The implementation introduces a new package
pkg/utilfor time formatting, which could create additional dependencies and coupling if not carefully managed. -
The nested command structure using cobra, while following CLI patterns, may increase complexity and learning curve for users interacting with the CLI.
-
The timeout management for non-follow mode may need careful review to ensure proper resource cleanup and avoid potential hanging processes.
Tip
⚡ Quick Actions
This review was generated by Mesa.
Actions:
Slash Commands:
/review- Request a full code review/review latest- Review only changes since the last review/describe- Generate PR description. This will update the PR body or issue comment depending on your configuration/help- Get help with Mesa commands and configuration options
4 files reviewed | 0 comments | Review on Mesa | Edit Reviewer Settings
masnwilliams
left a comment
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.
looking pretty solid to me
| return nil | ||
| } | ||
|
|
||
| table := pterm.TableData{{"Invocation ID", "App Name", "Action", "Status", "Started At", "Duration", "Output"}} |
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.
app version also useful
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.
in fact maybe even worth having the capability to filter on it in the API endpoint
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.
https://github.com/onkernel/kernel/pull/519
change for including version
Summary
This PR introduces two new features to enhance invocation management and debugging capabilities in the Kernel CLI:
kernel invoke history) - View historical invocations across all apps or filtered by a specific appkernel logs <app> --invocation <id>) - Stream logs for a specific invocation runChanges
New
kernel invoke historyCommand--appflag--limitflag (default: 100)Usage:
Enhanced Log Streaming
--invocationflag tokernel logscommand--followflag for continuous streamingUsage:
Other Improvements
FollowStreamingmethodTechnical Details
ListAutoPagingtoListto avoid pagination parsing issuesTesting
TL;DR
Adds
kernel invoke historyto view past invocations and enhanceskernel logswith an--invocationflag to stream logs for a specific run.Why we made these changes
To improve debugging and observability by allowing users to easily find and view logs for specific invocation runs, instead of searching through an entire app's log stream.
What changed?
cmd/invoke.go:invoke historysubcommand to list and filter recent invocations.invokecommand now prints the invocation ID upon submission for easier reference.cmd/logs.go:--invocation(-i) flag to stream logs for a specific invocation ID.go.mod&go.sum:kernel-go-sdkdependency fromv0.11.1tov0.11.4.Validation
Description generated by Mesa. Update settings