feat: add support for ttft #1161
Conversation
|
|
|
Hey @LastRemote it seems like you nee to sign CLA for https://github.com/deepset-ai/haystack-core-integrations - perhaps you have already? 🙏 |
No I actually haven't. But now it's done. |
There was a problem hiding this comment.
@LastRemote looks good but let's just handle this ValueError in case the time data is invalid
| span._span.update(usage=meta.get("usage") or None, model=meta.get("model")) | ||
| completion_start_time = meta.get("completion_start_time") | ||
| if completion_start_time: | ||
| completion_start_time = datetime.fromisoformat(completion_start_time) |
There was a problem hiding this comment.
fromisoformat throws ValueError for irregular format provided. Let's try/catch it, log it, and skip ttft in that case, wdyt?
There was a problem hiding this comment.
Sure, will do. Normally this won't be the case though.
There was a problem hiding this comment.
Yeah I agree, thanks for your understanding 👍
There was a problem hiding this comment.
Updated, and also added a new unit test for this.
github-actions
bot
added
the
type:documentation
LastRemote
force-pushed
the
dev/ttft-support
branch
from
fd80011 to
0ab5cc7
Compare
There was a problem hiding this comment.
Looks good @LastRemote - thanks for another great contribution!
Related Issues
Proposed Changes:
Support TTFT by casting isoformat to a datetime object and include it in a generation.
How did you test it?
Unit test. I also have E2E tests in my version, which is almost the same but not exactly.
Notes for the reviewer
Checklist
fix:,feat:,build:,chore:,ci:,docs:,style:,refactor:,perf:,test:.