Initial commit of Amazon Q Business runnable for langchain #301
Conversation
There was a problem hiding this comment.
Thank you for the contribution @tomron-aws ! Added a few comments.
Also, can you provide some additional context on what use-cases you're targeting with this ChatSync API integration?
Depending on your goals, it may be better to write this as a base Runnable interface, which can be simpler and more flexible to use than the specialized LLM interface that implements it.
Code changes should be minimal if you decide to switch (i.e. change _call/_acall to invoke/ainvoke, remove _llm_type and _identifying_params).
| print("Prompt Length (Amazon Q ChatSync API takes a maximum of 7000 chars)") | ||
| print(len(prompt)) |
There was a problem hiding this comment.
These look like debug print statements. Should they be removed or replaced with logger statements?
There was a problem hiding this comment.
I've been debating that. Currently Amazon Q ChatSync API only supports a max of 7000 chars. Its helpful to debug in the case that you are passing along additional context with the prompt.
I think a good place for that log would be in the error handling block, which I added. Let me know what you think
There was a problem hiding this comment.
Thanks - let's remove L83/84 in favor of the new changes.
| } | ||
|
|
||
| # Call Amazon Q | ||
| response = self.client.chat_sync(**request) |
There was a problem hiding this comment.
Before attempting to call chat_sync API, can we run some validation code to check if the QBusiness boto client has already been defined externally? If it doesn't exist, we should attempt to create it.
Refer here for an example: https://github.com/langchain-ai/langchain-aws/blob/main/libs/aws/langchain_aws/llms/sagemaker_endpoint.py#L271
There was a problem hiding this comment.
I implemented this as well. Let me know if that satisfies the requirements.
| logging.info(f"Prompt Length: {len(prompt)}") | ||
| print(f"""Prompt: | ||
| {prompt}""") | ||
| raise ValueError(f"Error raised by Amazon Q service: {e}") |
There was a problem hiding this comment.
Remove this line, as it's redundant with L123. Or change the ValueError message here to specifically reference the prompt length constraint
…te client using credentials sine anonymous invocation of chatsync with userId is no longer suppported
|
Changed to runnable. Also finished implementing and testing the client validator |
No description provided.