-
Notifications
You must be signed in to change notification settings - Fork 2
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: Add cache for deployment ID #96
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.
Pipeline seems to have caught some minor stuff, other than that LGTM
).toEqual('deployment-id1'); | ||
}); | ||
|
||
it('should ignore model versions without model name', () => { |
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.
any particular reason why? Ideally, this never happens, right?
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.
Yes, I wanted to make sure that we don't accidentally allow it though. The way we build the cache key this should not be possible anyways.
name: 'gpt-35-turbo', | ||
version: 'latest' |
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 would expect the test to pass with the following change, but it fails:
name: 'gpt-35-turbo', | |
version: 'latest' | |
name: 'gpt-35-turbo' |
Context
AI/gen-ai-hub-sdk-js-backlog#78.
Introduce a cache for deployment IDs.
I did not offer a possibility for users to change the default validity time, let me know if you see reasons to introduce this.
For langchain integration we will probably need to cache model information as well. I am wondering whether we should cache the ID + model or just the whole deployment directly. @MatKuhr @deekshas8 @tomfrenken
Definition of Done