-
Notifications
You must be signed in to change notification settings - Fork 38
feat: adding further support for cache control #144
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
feat: adding further support for cache control #144
Conversation
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.
Pull Request Overview
This PR enhances cache control functionality by adding support for different Time-To-Live (TTL) durations and providing visibility into cache creation metrics. The changes enable developers to specify cache duration (5 minutes or 1 hour) and receive detailed information about which cache types were created.
- Added TTL property to CacheControl class with predefined duration constants
- Added CacheCreation class to track ephemeral cache token usage by duration
- Updated tests to verify cache creation functionality with different TTL values
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Anthropic.SDK/Messaging/CacheControl.cs | Added TTL property and duration constants for cache control |
| Anthropic.SDK/Messaging/MessageResponse.cs | Added CacheCreation class to track cache usage by duration |
| Anthropic.SDK.Tests/DocumentTests.cs | Updated test to use TTL and verify cache creation metrics |
| Anthropic.SDK.Tests/CacheControlTests.cs | Enhanced tests with TTL usage and cache creation validation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Thanks for the PR! This library actually makes fairly heavy use of converters, would you consider using a converter for this? Some sample code I gen'd up: using System;
using System.Text.Json;
using System.Text.Json.Serialization;
public enum CacheDuration
{
FiveMinutes,
OneHour
}
public class CacheDurationConverter : JsonConverter<CacheDuration>
{
public override CacheDuration Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
string? value = reader.GetString();
return value switch
{
"5m" => CacheDuration.FiveMinutes,
"1h" => CacheDuration.OneHour,
_ => throw new JsonException($"Invalid cache duration: {value}")
};
}
public override void Write(Utf8JsonWriter writer, CacheDuration value, JsonSerializerOptions options)
{
string str = value switch
{
CacheDuration.FiveMinutes => "5m",
CacheDuration.OneHour => "1h",
_ => throw new JsonException($"Invalid cache duration enum: {value}")
};
writer.WriteStringValue(str);
}
}
using System.Text.Json.Serialization;
public class CacheControl
{
[JsonPropertyName("type")]
public CacheControlType Type { get; set; }
[JsonPropertyName("ttl")]
[JsonConverter(typeof(CacheDurationConverter))]
public CacheDuration TTL { get; set; }
} |
Co-authored-by: Copilot <[email protected]>
|
added the converter as requested, addressed the copilot concern. |
|
Awesome, thank you for the contribution, and the quick turn-around on feedback! |
This PR aims to add further visibility and control over caching.
Note: using a string with constants as C# doesnt like numbers in enum values and creating a custom converter is probably not something you want.