chore: Add README for MemoryManager #231
Conversation
jona62
had a problem deploying
to
auto-approve
GitHub Actions
Failure
| ) | ||
| ``` | ||
|
|
||
| ### Memory Limits and Quotas |
There was a problem hiding this comment.
Do we have the direct link to Memory Service limits ?
There was a problem hiding this comment.
I think that'll be available after GA
| for memory_summary in memories: | ||
| print(f"Memory: {memory_summary.id} - {memory_summary.name} ({memory_summary.status})") | ||
|
|
||
| # Get specific memory details |
There was a problem hiding this comment.
do we expect users to copy and use the code.. if so, is '-' an accepted char in memory id ?
There was a problem hiding this comment.
I think it is not valid, good callout
There was a problem hiding this comment.
memory id do have hyphens, memory names do not have hyphens. Also customers would not be able to copy and use this since memory_ids will be uniquely generated per memory resource
|
|
||
| ## Overview | ||
|
|
||
| The `MemoryManager` class provides a comprehensive interface for managing AWS Bedrock AgentCore Memory resources. It handles all **CONTROL PLANE CRUD operations** and supports both modern typed strategies (Pydantic models) and legacy dictionary-based configurations. |
There was a problem hiding this comment.
why is "CONTROL PLANE CRUD operations" in all caps -- are you mad at me?
There was a problem hiding this comment.
"supports both modern typed strategies (Pydantic models) and legacy dictionary-based configurations" that feels like a technical detail that is too deep in the weeds for this overview. I think the overview should be focused on: Here is what it is, here is what it does, here is why it is useful. I think we have the what it is and does, but not really the why. I think making the final sentence here a brief 1 or 2 sentences that say why AgentCore Memory is useful would be great (since many of the people that visit this will be hearing of AgentCore memory for the first time)
There was a problem hiding this comment.
and if we don't explain the terminology, we should at least link to something that does
| retries={'max_attempts': 3} | ||
| ) | ||
| manager = MemoryManager(region_name="us-east-1", boto_client_config=custom_config_with_agent) | ||
| # Final user agent will be: "my-application/1.0 bedrock-agentcore-starter-toolkit" |
There was a problem hiding this comment.
this is wrong, right? It will include a bunch of stuff about boto3 (but this shouldn't matter to the end user)
There was a problem hiding this comment.
I think we should remove user_agent_extra from these examples as it seems only relevant to the service owner for telemetry/debugging purposes
| for memory_summary in memories: | ||
| print(f"Memory: {memory_summary.id} - {memory_summary.name} ({memory_summary.status})") | ||
|
|
||
| # Get specific memory details |
There was a problem hiding this comment.
I think it is not valid, good callout
|
|
||
| --- | ||
|
|
||
| ### Memory Lifecycle Methods |
There was a problem hiding this comment.
I somewhat think we shouldn't do this unless we have tests that verify that changes to the code also update this. I also think Python does this for you. I remember Nikhil showing me how you can do something with Python that shows you all the doc strings and functions in a given class (I don't remember how right now, but that should replace this so that we don't have to maintain these separate things and can instead just rely on Python)
There was a problem hiding this comment.
Yeah there are doc strings in each method which show the same, it's kind of redundant to have this, removing!
jona62
force-pushed
the
chore/add-readme-memory-manager
branch
from
97b15c3 to
ab8ef02
Compare
jona62
force-pushed
the
chore/add-readme-memory-manager
branch
from
ab8ef02 to
6c5869c
Compare
Description
Add README for MemoryManager
Type of Change
Testing
Checklist
Security Checklist
Breaking Changes
List any breaking changes and migration instructions:
N/A
Additional Notes
N/A