-
Notifications
You must be signed in to change notification settings - Fork 547
update for Dapr 1.16 conversation SDK Python quickstart #1215
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
base: release-1.16
Are you sure you want to change the base?
Conversation
Signed-off-by: Filinto Duran <[email protected]>
@filintod not sure we want to put this example in quickstarts otherwise it runs as a test and we dont have an API key to use? we could talk about putting it in https://github.com/dapr/quickstarts/tree/master/tutorials ? |
@alicejgibbons make sense, some updates are still ok and use the echo provider, I guess we can move the API-dependant parts to the tutorial. I have created a lengthier example one in python-sdk that I could bring to the tutorial instead, like a multi-turn example that is more interesting. |
@filintod here's what i would propose:
|
conversation/python/http/README.md
Outdated
@@ -24,7 +24,34 @@ name: Install Python dependencies | |||
|
|||
```bash | |||
cd ./conversation | |||
``` | |||
|
|||
<details open="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.
Not opposed to adding venv details but Sam does have this qs open here: #1208
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'll rebase to work on top of that as it is now merged
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.
there are still some contrib changes required.
|
||
2. Open a new terminal window and run the multi app run template: | ||
|
||
<!-- STEP |
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.
We need to keep all the tests in, you can run make validate
locally to check
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 kept the steps further down but I'll check
from dapr.clients.grpc._request import (ConversationInputAlpha2, ConversationMessage, ConversationMessageContent, | ||
ConversationMessageOfUser, ConversationToolsFunction, ConversationTools) | ||
|
||
with DaprClient() as d: |
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.
Lets keep the tool calling sample in the same file as the basic python qs so that there is only one app to run as Constantin did here: #1209
The mechanical markdown test should test the same input/output as well:
== APP - conversation == Tool calling input sent: What is the weather like in San Francisco in celsius?
== APP - conversation == Output message: What is the weather like in San Francisco in celsius?
== APP - conversation == No tool calls in response
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 updated this as the outputs are outdated from latest changes in contrib and expected PRs there
#dapr>=1.15.0 | ||
|
||
-e ../../../../../python-sdk |
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.
We need to set the version per quickstart and this should pull in the latest python sdk rc: v1.16.0rc1
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, this is all temporary to work in my computer, we need python sdk with my changes that is not merged yet
Signed-off-by: Filinto Duran <[email protected]>
Signed-off-by: Filinto Duran <[email protected]>
@alicejgibbons I removed the openai/ollama references and I'll add to tutorials after this is done, and also the Python SDK PR is done (required for this one). |
Description
Please explain the changes you've made
Issue reference
We strive to have all PRs being opened based on an issue, where the problem or feature have been discussed prior to implementation.
#1157
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: