-
Notifications
You must be signed in to change notification settings - Fork 40
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
Add function_call_id to AppGetLogsRequest #2795
Conversation
modal_proto/api.proto
Outdated
@@ -2598,6 +2599,7 @@ message TaskLogsBatch { | |||
string image_id = 13; // Used for image logs | |||
bool eof = 14; | |||
string pty_exec_id = 15; // Used for interactive functions | |||
string function_call_id = 16; |
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 we need this here since we already have a have the function_call_id
on TaskLogs? Might be mistaken
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.
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.
(Could be wrong too)
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.
We changed this line to filter on the item instead of batch, so removed function_call_id
from TaskLogsBatch
.
@prbot approve |
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.
Approved 👍. @mwaskom will follow-up review this.
Nice, I think users have asked about filtering |
Required to update
/logs-stream
RPC here@mwaskom we are adding the ability to filter logs by
functionCallId
to need this as a new filter on this RPC. The protobuf types live in the client, so wanted to run by you first. LMK if you have ?'s.