feat(groq): Migrate Groq to v2 Plugins API #348
Conversation
|
you will need to implement the changes see this similar PR: #347 |
plugins/groq/src/index.ts
Outdated
| }, | ||
| list: async () => { | ||
| return Object.keys(SUPPORTED_GROQ_MODELS).map((name) => ({ | ||
| name: `groq/${name}`, |
There was a problem hiding this comment.
Well, groq/ was initially being appended to the model name.
The prefix groq/ was added here:
https://github.com/BloomLabsInc/genkit-plugins/pull/9/files#diff-9c9efbee9ae1c4a79760d6d2d0f89c9d46d538f645235c21e3f51f3c4c9e35aeR454-R457
When the initialized Genkit object is logged while using the genkitx-groq package (which has the v1 plugin under the hood), the resulting model name will include this prefix.
There was a problem hiding this comment.
I don't think it's necessary anymore in v2.
There was a problem hiding this comment.
alright, removing it shortly
There was a problem hiding this comment.
@HassanBahati I think we're going for namespace: 'pluginName' and name: 'modelName' instead. So can we apply that here and to your other migrations too.
For example:
{
name: 'llama3-70b-8192',
namespace: 'groq'
...
}There was a problem hiding this comment.
Thanks @CorieW, i've updated
| @@ -97,3 +98,45 @@ describe('toGroqRequestBody', () => { | |||
| }); | |||
| }); | |||
| }); | |||
|
|
|||
| describe('Groq Plugin', () => { | |||
| it('should create plugin with v2 API', () => { | |||
There was a problem hiding this comment.
@cabljac thoughts on this test?
There was a problem hiding this comment.
it's not bad, but tests seem quite minimal. ideally i'd like some integration tests with the core genkit methods
e.g we initialise genkit with the plugin, we mock global fetch, we call ai.generate with groq
4 participants
Before you submit a pull request, please make sure you have read and understood the contribution guidelines and the code of conduct.
This pull request is related to:
I have checked the following:
Description:
Please provide a clear and concise description of the changes you are proposing here.
Related issues:
Closes #353