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

OllamaSharp registration fixes #318

Merged
merged 2 commits into from
Dec 11, 2024
Merged

OllamaSharp registration fixes #318

merged 2 commits into from
Dec 11, 2024

Conversation

aaronpowell
Copy link
Member

Refactor OllamaSharp client so that the MEAI types will resolve the correct OllamaSharp API and thus the right default model.

Fixes #310

Closes #<ISSUE_NUMBER>

PR Checklist

  • Created a feature/dev branch in your fork (vs. submitting directly from a commit on main)
  • Based off latest main branch of toolkit
  • PR doesn't include merge commits (always rebase on top of our main, if needed)
  • New integration
    • Docs are written
    • Added description of major feature to project description for NuGet package (4000 total character limit, so don't push entire description over that)
  • Tests for the changes have been added (for bug fixes / features) (if applicable)
  • Contains NO breaking changes
  • Every new API (including internal ones) has full XML docs
  • Code follows all style conventions

Other information

Copy link

github-actions bot commented Dec 9, 2024

Code Coverage

Package Line Rate Branch Rate Complexity Health
CommunityToolkit.Aspire.EventStore 100% 100% 46
CommunityToolkit.Aspire.Hosting.ActiveMQ 60% 24% 104
CommunityToolkit.Aspire.Hosting.ActiveMQ.MassTransit 1% 0% 14
CommunityToolkit.Aspire.Hosting.Azure.DataApiBuilder 100% 100% 22
CommunityToolkit.Aspire.Hosting.Azure.StaticWebApps 100% 100% 28
CommunityToolkit.Aspire.Hosting.Bun 81% 71% 54
CommunityToolkit.Aspire.Hosting.Deno 84% 75% 72
CommunityToolkit.Aspire.Hosting.EventStore 90% 71% 62
CommunityToolkit.Aspire.Hosting.Golang 94% 50% 16
CommunityToolkit.Aspire.Hosting.Java 98% 71% 58
CommunityToolkit.Aspire.Hosting.Meilisearch 61% 27% 94
CommunityToolkit.Aspire.Hosting.NodeJS.Extensions 90% 68% 92
CommunityToolkit.Aspire.Hosting.Ollama 65% 64% 198
CommunityToolkit.Aspire.Hosting.Python.Extensions 67% 46% 56
CommunityToolkit.Aspire.Hosting.Rust 94% 83% 16
CommunityToolkit.Aspire.Hosting.SqlDatabaseProjects 72% 64% 58
CommunityToolkit.Aspire.MassTransit.RabbitMQ 100% 100% 30
CommunityToolkit.Aspire.Meilisearch 97% 92% 68
CommunityToolkit.Aspire.OllamaSharp 87% 82% 68
Summary 76% (2319 / 3046) 63% (577 / 912) 1156

Minimum allowed line rate is 60%

@aaronpowell aaronpowell requested a review from Copilot December 11, 2024 23:45

Choose a reason for hiding this comment

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

Copilot reviewed 3 out of 3 changed files in this pull request and generated no suggestions.

Comments skipped due to low confidence (1)

tests/CommunityToolkit.Aspire.OllamaSharp.Tests/ConformanceTests.cs:33

  • The use of 'string?' in 'KeyValuePair<string, string?>' is unnecessary if the values are always provided and never null. Consider changing it to 'string'.
new KeyValuePair<string, string?>("ConnectionStrings:{key ?? 'ollama'}", $"Endpoint={endpoint}"),
@aaronpowell aaronpowell merged commit 6f99735 into main Dec 11, 2024
10 checks passed
@aaronpowell aaronpowell deleted the aaronpowell/issue310 branch December 11, 2024 23:46
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.

Multiple registrations of the OllamaSharp integration always uses the first one
1 participant