-
Notifications
You must be signed in to change notification settings - Fork 760
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
Usage aggregation via Dictionary<string, long> #5709
Conversation
🎉 Good job! The coverage increased 🎉
Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=883335&view=codecoverage-tab |
src/Libraries/Microsoft.Extensions.AI.Abstractions/UsageDetails.cs
Outdated
Show resolved
Hide resolved
src/Libraries/Microsoft.Extensions.AI.Abstractions/AdditionalPropertiesDictionary.cs
Outdated
Show resolved
Hide resolved
src/Libraries/Microsoft.Extensions.AI.Abstractions/AdditionalPropertiesDictionary{TValue}.cs
Outdated
Show resolved
Hide resolved
src/Libraries/Microsoft.Extensions.AI.Abstractions/AdditionalPropertiesDictionary{TValue}.cs
Outdated
Show resolved
Hide resolved
🎉 Good job! The coverage increased 🎉
Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=885946&view=codecoverage-tab |
🎉 Good job! The coverage increased 🎉
Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=886016&view=codecoverage-tab |
This is a much simpler version of #5707 in case we prefer this approach. It limits additional usage data to be of type
long
, with the payoff that we require much less implementation and new public API surface.If we think this will suffice, it's certainly advantageous vs #5707. Arguably we could proceed with this as-is, and then in the future if further "additional usage" value types are needed, we'd still have the option to add some other property called
AdditionalValues
(or similar) toUsageDetails
which accepts a wider range of types.Microsoft Reviewers: Open in CodeFlow