-
Notifications
You must be signed in to change notification settings - Fork 1.4k
.NET: Add container file download extension for Code Interpreter cfile_ IDs #5014
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: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| // Copyright (c) Microsoft. All rights reserved. | ||
|
|
||
| using System.ClientModel; | ||
| using System.Diagnostics.CodeAnalysis; | ||
| using Microsoft.Shared.DiagnosticIds; | ||
| using Microsoft.Shared.Diagnostics; | ||
|
|
||
| namespace OpenAI; | ||
|
|
||
| /// <summary> | ||
| /// Provides extension methods for <see cref="OpenAIClient"/> to download files from OpenAI containers. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// Container files are generated by tools such as Code Interpreter and have IDs prefixed with | ||
| /// <c>cfile_</c>. These files reside within a specific container (identified by a <c>cntr_</c> | ||
| /// prefixed ID) and cannot be downloaded using the standard Files API | ||
| /// (<see cref="OpenAIClient.GetOpenAIFileClient"/>). Use these methods to download container-scoped | ||
| /// files through the Containers API instead. | ||
| /// </remarks> | ||
| [Experimental(DiagnosticIds.Experiments.AIOpenAIResponses)] | ||
| public static class OpenAIClientContainerFileExtensions | ||
| { | ||
| /// <summary> | ||
| /// Downloads a file from an OpenAI container. | ||
| /// </summary> | ||
| /// <param name="client">The <see cref="OpenAIClient"/> to use for the download.</param> | ||
| /// <param name="containerId">The ID of the container (e.g., <c>cntr_...</c>) that holds the file.</param> | ||
| /// <param name="fileId">The ID of the file to download (e.g., <c>cfile_...</c>).</param> | ||
| /// <param name="cancellationToken">The <see cref="CancellationToken"/> to monitor for cancellation requests. The default is <see cref="CancellationToken.None"/>.</param> | ||
| /// <returns>A <see cref="Task{ClientResult{BinaryData}}"/> containing the downloaded file data as <see cref="BinaryData"/>.</returns> | ||
|
Check failure on line 30 in dotnet/src/Microsoft.Agents.AI.OpenAI/Extensions/OpenAIClientContainerFileExtensions.cs
|
||
| /// <exception cref="ArgumentNullException">Thrown when <paramref name="client"/>, <paramref name="containerId"/>, or <paramref name="fileId"/> is <see langword="null"/>.</exception> | ||
| /// <exception cref="ArgumentException">Thrown when <paramref name="containerId"/> or <paramref name="fileId"/> is empty or consists only of whitespace characters.</exception> | ||
| public static async Task<ClientResult<BinaryData>> DownloadContainerFileAsync( | ||
| this OpenAIClient client, | ||
| string containerId, | ||
| string fileId, | ||
| CancellationToken cancellationToken = default) | ||
| { | ||
| Throw.IfNull(client); | ||
| Throw.IfNullOrWhitespace(containerId); | ||
| Throw.IfNullOrWhitespace(fileId); | ||
|
|
||
| var containerClient = client.GetContainerClient(); | ||
| return await containerClient.DownloadContainerFileAsync(containerId, fileId, cancellationToken).ConfigureAwait(false); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,132 @@ | ||
| // Copyright (c) Microsoft. All rights reserved. | ||
|
|
||
| using System; | ||
| using System.Threading.Tasks; | ||
| using OpenAI; | ||
|
|
||
| namespace Microsoft.Agents.AI.OpenAI.UnitTests.Extensions; | ||
|
|
||
| /// <summary> | ||
| /// Unit tests for the <see cref="OpenAIClientContainerFileExtensions"/> class. | ||
| /// </summary> | ||
| public sealed class OpenAIClientContainerFileExtensionsTests | ||
| { | ||
| /// <summary> | ||
| /// Verify that DownloadContainerFileAsync throws ArgumentNullException when client is null. | ||
| /// </summary> | ||
| [Fact] | ||
| public async Task DownloadContainerFileAsync_WithNullClient_ThrowsArgumentNullExceptionAsync() | ||
| { | ||
| // Act & Assert | ||
| var exception = await Assert.ThrowsAsync<ArgumentNullException>( | ||
| () => ((OpenAIClient)null!).DownloadContainerFileAsync("cntr_123", "cfile_456")); | ||
|
|
||
| Assert.Equal("client", exception.ParamName); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Verify that DownloadContainerFileAsync throws ArgumentNullException when containerId is null. | ||
| /// </summary> | ||
| [Fact] | ||
| public async Task DownloadContainerFileAsync_WithNullContainerId_ThrowsArgumentNullExceptionAsync() | ||
| { | ||
| // Arrange | ||
| var client = new TestOpenAIClient(); | ||
|
|
||
| // Act & Assert | ||
| var exception = await Assert.ThrowsAsync<ArgumentNullException>( | ||
| () => client.DownloadContainerFileAsync(null!, "cfile_456")); | ||
|
|
||
| Assert.Equal("containerId", exception.ParamName); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Verify that DownloadContainerFileAsync throws ArgumentException when containerId is empty. | ||
| /// </summary> | ||
| [Fact] | ||
| public async Task DownloadContainerFileAsync_WithEmptyContainerId_ThrowsArgumentExceptionAsync() | ||
| { | ||
| // Arrange | ||
| var client = new TestOpenAIClient(); | ||
|
|
||
| // Act & Assert | ||
| var exception = await Assert.ThrowsAsync<ArgumentException>( | ||
| () => client.DownloadContainerFileAsync("", "cfile_456")); | ||
|
|
||
| Assert.Equal("containerId", exception.ParamName); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Verify that DownloadContainerFileAsync throws ArgumentException when containerId is whitespace. | ||
| /// </summary> | ||
| [Fact] | ||
| public async Task DownloadContainerFileAsync_WithWhitespaceContainerId_ThrowsArgumentExceptionAsync() | ||
| { | ||
| // Arrange | ||
| var client = new TestOpenAIClient(); | ||
|
|
||
| // Act & Assert | ||
| var exception = await Assert.ThrowsAsync<ArgumentException>( | ||
| () => client.DownloadContainerFileAsync(" ", "cfile_456")); | ||
|
|
||
| Assert.Equal("containerId", exception.ParamName); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Verify that DownloadContainerFileAsync throws ArgumentNullException when fileId is null. | ||
| /// </summary> | ||
| [Fact] | ||
| public async Task DownloadContainerFileAsync_WithNullFileId_ThrowsArgumentNullExceptionAsync() | ||
| { | ||
| // Arrange | ||
| var client = new TestOpenAIClient(); | ||
|
|
||
| // Act & Assert | ||
| var exception = await Assert.ThrowsAsync<ArgumentNullException>( | ||
| () => client.DownloadContainerFileAsync("cntr_123", null!)); | ||
|
|
||
| Assert.Equal("fileId", exception.ParamName); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Verify that DownloadContainerFileAsync throws ArgumentException when fileId is empty. | ||
| /// </summary> | ||
| [Fact] | ||
| public async Task DownloadContainerFileAsync_WithEmptyFileId_ThrowsArgumentExceptionAsync() | ||
| { | ||
| // Arrange | ||
| var client = new TestOpenAIClient(); | ||
|
|
||
| // Act & Assert | ||
| var exception = await Assert.ThrowsAsync<ArgumentException>( | ||
| () => client.DownloadContainerFileAsync("cntr_123", "")); | ||
|
|
||
| Assert.Equal("fileId", exception.ParamName); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Verify that DownloadContainerFileAsync throws ArgumentException when fileId is whitespace. | ||
| /// </summary> | ||
| [Fact] | ||
| public async Task DownloadContainerFileAsync_WithWhitespaceFileId_ThrowsArgumentExceptionAsync() | ||
| { | ||
| // Arrange | ||
| var client = new TestOpenAIClient(); | ||
|
|
||
| // Act & Assert | ||
| var exception = await Assert.ThrowsAsync<ArgumentException>( | ||
| () => client.DownloadContainerFileAsync("cntr_123", " ")); | ||
|
|
||
| Assert.Equal("fileId", exception.ParamName); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// A test <see cref="OpenAIClient"/> subclass for unit testing without requiring an API key. | ||
| /// </summary> | ||
| private sealed class TestOpenAIClient : OpenAIClient | ||
| { | ||
| public TestOpenAIClient() | ||
| { | ||
| } | ||
| } | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
| } | ||
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.
Current tests only validate argument guards; there’s no coverage that a valid call actually routes through
GetContainerClient().DownloadContainerFileAsync(...)(the core behavior this PR adds). Consider adding a positive-path test using a custom transport / pipeline to assert the request goes to the Containers API endpoint and succeeds (without requiring real network calls).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.
@copilot apply changes based on this feedback