-
Notifications
You must be signed in to change notification settings - Fork 401
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
feat: Added AIM support for Meta Llama3 models in AWS Bedrock #2306
Conversation
amychisholm03
commented
Jun 26, 2024
- Will now just check if the model name starts with meta-llama, not meta-llama2
- Modified unit and versioned:major tests to test both llama2 and llama3, or just llama if applicable
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.
Looks great. Just have a question for clarification.
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.
A few suggestions to reduce the amount of test code we'd have to maintain. the llama2/3 mocks are identical and prob unnecessary. you could consolidate it and update the versioned tests. I also plan to reach out and discuss some git commit tips as this branch has a lot of superfluous commits in it
@@ -119,6 +119,12 @@ function handler(req, res) { | |||
break | |||
} | |||
|
|||
case 'meta.llama3-8b-instruct-v1:0': | |||
case 'meta.llama3-70b-instruct-v1:0': { | |||
response = responses.llama3.get(payload.prompt) |
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.
since llama3 responses are identical you don't really need mocks. you could just return the llama2 data
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.
It does make sense to utilize the existing mocks. 👍
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.
Please update https://github.com/newrelic/node-newrelic/blob/main/ai-support.json
https://github.com/newrelic/node-newrelic/blob/main/ai-support.spec.md should explain everything, but please ask me if you have questions.
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.
🚢 🇮🇹