-
Notifications
You must be signed in to change notification settings - Fork 19
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
chore: send logs query param to status request as true by default #19
Conversation
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.
Cool, thanks for doing this!
The question I have is, if the logs are disabled, does the queue status payload still contain an empty logs
property? Or is it missing or null
?
Asking because queue status is defined like this, with logs
always present in case it's IN_PROGRESS
or COMPLETED
, so is this still correct?
export type QueueStatus =
| {
status: 'IN_PROGRESS' | 'COMPLETED';
response_url: string;
logs: RequestLog[];
}
| {
status: 'IN_QUEUE';
queue_position: number;
response_url: string;
};
Ah, another recommendation I have is to add the option to the Related to that, is the idea to keep as |
Co-authored-by: Daniel Rochetti <[email protected]>
I just wanted to make our client more backwards compatible. I can do whichever |
got it! I think if the idea is to make logs only available on demand, we can make it |
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.
Awesooome 🚀
We are making getting the logs during a status request optional. So enabling this to get them (or not) as necessary.
closes FEA-1645