Skip to content
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

Added Dapr Conversation .NET Quickstart #1133

Closed
wants to merge 5 commits into from

Conversation

WhitWaldo
Copy link
Contributor

Description

Added quickstart featuring the Dapr Conversation API using Anthropic.

Issue reference

N/A - part of 1.15 endgame

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • The quickstart code compiles correctly
  • You've tested new builds of the quickstart if you changed quickstart code
  • You've updated the quickstart's README if necessary
  • If you have changed the steps for a quickstart be sure that you have updated the automated validation accordingly. All of our quickstarts have annotations that allow them to be executed automatically as code. For more information see mechanical-markdown. For user guide with examples see Examples.

Copy link

@alicejgibbons alicejgibbons left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @WhitWaldo, a couple of things here:

@WhitWaldo
Copy link
Contributor Author

Thanks @WhitWaldo, a couple of things here:

* can you remove the python sdk crypto quickstart commits since we are cherrypicking those into the release here: [Cherry picking 42a99c2 into release branch #1135](https://github.com/dapr/quickstarts/pull/1135)

* This quickstart is more complex than some of the other ones we were going to do! We were just going to use the basic `echo` dapr component, like you did in your dotnet example: https://github.com/dapr/dotnet-sdk/blob/release-1.15-rc01/examples/AI/ConversationalAI/Program.cs

* can you sign off your commits :)

It seems the python commits get added automatically when I target merging with dapr:release-1.15 as I don't have that commit in my fork. Should I target a different branch to merge into?

This isn't any more complicated than the version in the .NET examples in that it's just sending a message to the AI endpoint and returning the response. I do think that an example should include best practices (e.g. I can't stand those examples that say don't use passwords in your code, then proceed to do exactly that) - here, this example prefers DI registration over the static builder and the current best practice around .NET logging.

I believe my commit is properly signed here.

Copy link
Contributor

@paulyuk paulyuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see comment


<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>net9.0</TargetFramework>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need net 9? I'm thinking we could stay on .NEt 8 because it's LTS, it's on more dev boxes, and that's whats in our test env unless we take action to change the GH actions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also please sync to release-1.15 and attempt to get C# checks to pass?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can stay on .NET 8 here if you'd like. We've got support for .NET 6, 8 and 9 with 1.15 for all clients except Dapr.Workflow (which supports .NET 7, 8 and 9). Version 1.16 will drop .NET 6 and .NET 7.

@paulyuk
Copy link
Contributor

paulyuk commented Jan 24, 2025

Using the joint collab found in PR #1144

@paulyuk paulyuk closed this Jan 24, 2025
@WhitWaldo WhitWaldo deleted the conversation-dotnet branch January 24, 2025 00:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants