-
Notifications
You must be signed in to change notification settings - Fork 902
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
docs: update rag.md example code to prevent errors #1009
Conversation
instructions="You are a helpful assistant", | ||
enable_session_persistence=True, |
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.
was this necessary? this should not be necessary for the code to work right?
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 not need to be set to True
, but it does seem to need to be present. If I comment it out and run the same code I see the following error:
llama_stack_client.BadRequestError: Error code: 400 - {'error': {'detail': {'errors': [{'loc': ['body', 'agent_config', 'enable_session_persistence'], 'msg': 'Field required', 'type': 'missing'}]}}}
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.
interesting that's good feedback. could you set it to False
please for this doc?
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.
sure 😃
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.
I am fixing this in #1012
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.
@terrytangyuan is it worth removing from the example code then?
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.
Well the example doesn't work for the current released version so it might be nice to set it to false here so it works across versions
Signed-off-by: Michael Clifford <[email protected]>
@ashwinb @terrytangyuan Any more changes you'd like to see in this PR? 😃 |
) # What does this PR do? This issue was discovered in #1009 (comment). ## Test Plan This field is no longer required after the change. [//]: # (## Documentation) [//]: # (- [ ] Added a Changelog entry if the change is significant) --------- Signed-off-by: Yuan Tang <[email protected]> Co-authored-by: Ashwin Bharambe <[email protected]>
…ta-llama#1012) # What does this PR do? This issue was discovered in meta-llama#1009 (comment). ## Test Plan This field is no longer required after the change. [//]: # (## Documentation) [//]: # (- [ ] Added a Changelog entry if the change is significant) --------- Signed-off-by: Yuan Tang <[email protected]> Co-authored-by: Ashwin Bharambe <[email protected]>
What does this PR do?
This PR updates the example code in
docs/source/building_applications/rag.md
to reflect some changes in the SDK that were causing errors. It also adds a couple import statements to make things slightly easier for users.Test Plan
I've run each code segment using both
llamastack/distribution-ollama:latest
andllama stack run ollama
as my local llamastack servers withmeta-llama/Llama-3.2-3B-Instruct
as the inference model. All code segments ran without errors in both cases.