You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Custom deployment of LLMs sometimes require parameters that either don't exist in the crewai.LLM class parameters or require different naming conventions.
This change adds the capability to pass in custom parameters to the crewai.LLM class which then forwards them on for LiteLLM completion. Usage would look like the following:
Disclaimer: This review was made by a crew of AI Agents.
Code Review Comment for SageMaker LLM Provider Implementation
Overview
This pull request introduces Amazon SageMaker as a new LLM (Large Language Model) provider within CrewAI, encompassing both documentation and code modifications across multiple files.
Documentation Changes (docs/concepts/llms.mdx)
Strengths:
The documentation structure follows existing conventions and clearly outlines the setup process for SageMaker.
Includes comprehensive examples for configuring SageMaker, which is beneficial for new users.
Successfully removed duplicated AWS credentials handling between Bedrock and SageMaker, improving clarity and reducing redundancy.
Cleaned up unnecessary constants, enhancing maintainability.
Improvement Suggestions:
Consider implementing a shared AWS credentials handler across providers to streamline user input processes:
# src/crewai/cli/constants.pyAWS_COMMON_CREDENTIALS= [
{
"prompt": "Enter your AWS Access Key ID (press Enter to skip)",
"key_name": "AWS_ACCESS_KEY_ID",
},
{
"prompt": "Enter your AWS Secret Access Key (press Enter to skip)",
"key_name": "AWS_SECRET_ACCESS_KEY",
},
{
"prompt": "Enter your AWS Region Name (press Enter to skip)",
"key_name": "AWS_REGION_NAME",
},
]
PROVIDER_CONFIGS= {
"bedrock": AWS_COMMON_CREDENTIALS,
"sagemaker": AWS_COMMON_CREDENTIALS,
}
LLM Class Changes (src/crewai/llm.py)
Strengths:
Clean implementation of custom parameters provides flexibility while preserving backwards compatibility.
Key Issues Identified:
Potential for KeyError when accessing custom parameters needs addressing.
Missing type hints for custom_params can cause confusion about expected data types.
Lack of validation for custom parameters could lead to unexpected behavior.
Recommended Improvements:
To enhance flexibility and reliability, consider implementing error handling and validation for custom parameters:
Implement validation for custom parameters to prevent injection attacks.
Consider logging for custom parameter usage to facilitate debugging.
Performance Considerations
Implement caching for AWS credential resolution.
Consider connection pooling for SageMaker endpoints.
Improve timeout management for SageMaker calls.
Testing Recommendations
Develop unit tests for custom parameter handling.
Include integration tests with SageMaker endpoints.
Add error handling tests for invalid configurations.
Overall Assessment
The implementation is solid but could benefit from additional error handling and type safety. The removal of duplicate AWS credentials handling is a significant positive change that enhances maintainability. Implementing the suggested improvements would further boost reliability and usability while maintaining the integrity of the existing code. These changes lay a strong foundation for SageMaker integration in CrewAI.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Custom deployment of LLMs sometimes require parameters that either don't exist in the
crewai.LLMclass parameters or require different naming conventions.This change adds the capability to pass in custom parameters to the
crewai.LLMclass which then forwards them on for LiteLLM completion. Usage would look like the following: