Add Bedrock Provider Quickstart Notebook - #2560
Conversation
dosubot
Bot
added
the
size:M
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
| @@ -0,0 +1,298 @@ | |||
| { | |||
There was a problem hiding this comment.
| @@ -0,0 +1,298 @@ | |||
| { | |||
There was a problem hiding this comment.
no need to set regular key variables here and then print them, just keep them as environment variables and pass the env variables in the later cells.
Reply via ReviewNB
| @@ -0,0 +1,298 @@ | |||
| { | |||
There was a problem hiding this comment.
don't leave this ouput in the notebook - also this makes me worry you haven't run this example successfully. please verify the example runs successfully end to end.
Reply via ReviewNB
…/github.com/RamanaReddyGunda/trulens into ramana/bedrock-provider-quickstart-notebook
| @@ -0,0 +1,288 @@ | |||
| { | |||
There was a problem hiding this comment.
Line #5. os.environ['TRULENS_OTEL_TRACING'] = '0'
don't turn off OTEL tracing - why did you do this to start with? Did you run into errors?
Reply via ReviewNB
| @@ -0,0 +1,288 @@ | |||
| { | |||
There was a problem hiding this comment.
Line #1. try:
Call an actual out of the box metric (such as relevance) rather than building your own with the hidden _create_chat_completion method.
Reply via ReviewNB
| @@ -0,0 +1,288 @@ | |||
| { | |||
There was a problem hiding this comment.
Line #9. def context_relevance(query: str, context: List[Dict] = None) -> Dict[str, Any]:
use out of the box context relevance metric instead of creating your own.
Reply via ReviewNB
| @@ -0,0 +1,288 @@ | |||
| { | |||
There was a problem hiding this comment.
| @@ -0,0 +1,288 @@ | |||
| { | |||
There was a problem hiding this comment.
Line #10. record_id = session.add_record(
create an app and instrument it like in the trulens quickstart (you are welcome to use this simple retrieve function in it) instead of doing add_record.
Reply via ReviewNB
| @@ -0,0 +1,288 @@ | |||
| { | |||
There was a problem hiding this comment.
Line #19. meta={'model': bedrock_model},
the bedrock model should be captured automatically once you add in instrumentatino so no need to manually pass it here
Reply via ReviewNB
| @@ -0,0 +1,288 @@ | |||
| { | |||
There was a problem hiding this comment.
Split up the cell descriptions to above each cell they describe instead of including here at the end.
Reply via ReviewNB
sfc-gh-jreini
left a comment
sfc-gh-jreini
left a comment
There was a problem hiding this comment.
Progress: built-in metrics (provider.relevance, provider.context_relevance) and TruBasicApp are the right patterns — those match the Anthropic and other provider quickstarts.
Remaining issue: the rag_app function calls provider._create_chat_completion() directly. That is a private method on TruLens's LLMProvider base class — it is not part of the public API and can change without notice. Quickstart notebooks should not teach users to call private methods.
For the generation step, call the Bedrock SDK directly, the same way the Anthropic quickstart calls anthropic.completions.create():
import boto3
bedrock_client = boto3.client('bedrock-runtime', region_name=aws_region)
response = bedrock_client.converse(
modelId=bedrock_model,
messages=[{'role': 'user', 'content': [{'text': prompt}]}]
)
return response['output']['message']['content'][0]['text']
This is one focused change — everything else looks good.
|
Checking in — the Aug 10 commit looks like just a sync merge from main, not the fix I flagged. The outstanding item is still: replace |
…/github.com/RamanaReddyGunda/trulens into ramana/bedrock-provider-quickstart-notebook
|
my latest code pushed branch (https://github.com/RamanaReddyGunda/trulens/tree/ramana/bedrock-provider-quickstart-notebook) and raised merge request to main branch. |
3 participants
Closes #2469
Description
Please include a summary of the changes and the related issue that can be
included in the release announcement. Please also include relevant motivation
and context.
Other details good to know for developers
Please include any other details of this change useful for TruLens developers.
Type of change
not work as expected)