-
Notifications
You must be signed in to change notification settings - Fork 1k
.NET: [Breaking] Allow passing auth token credential to cosmosdb extensions #3250
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?
.NET: [Breaking] Allow passing auth token credential to cosmosdb extensions #3250
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 updates CosmosDB extension methods to accept TokenCredential parameters for authentication instead of hardcoding DefaultAzureCredential, providing more flexibility for production scenarios. This is a breaking change as it modifies the method signatures.
Changes:
- Replaced
Azure.Identitynamespace import withAzure.Corein both extension files - Added
TokenCredentialparameter toCreateCheckpointStoreUsingManagedIdentitymethods (both generic and non-generic) - Added
TokenCredentialparameter toWithCosmosDBMessageStoreUsingManagedIdentitymethod - Added null validation for the new
tokenCredentialparameter in all modified methods
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| CosmosDBWorkflowExtensions.cs | Added tokenCredential parameter to both CreateCheckpointStoreUsingManagedIdentity overloads with validation |
| CosmosDBChatExtensions.cs | Added tokenCredential parameter to WithCosmosDBMessageStoreUsingManagedIdentity with validation |
dotnet/src/Microsoft.Agents.AI.CosmosNoSql/CosmosDBWorkflowExtensions.cs
Show resolved
Hide resolved
dotnet/src/Microsoft.Agents.AI.CosmosNoSql/CosmosDBWorkflowExtensions.cs
Show resolved
Hide resolved
dotnet/src/Microsoft.Agents.AI.CosmosNoSql/CosmosDBChatExtensions.cs
Outdated
Show resolved
Hide resolved
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.
We should also consider letting the user provide a CosmosClient directly; I imagine people building webapps will store more data than just the chat history, and it is likely that they will reuse a CosmosDB account for it, which will likely be configured on DI.
We actually have a work item (#2982) for this already and @TheovanKraay, who contributed these implementations, said that he can take a look. |
|
I agree with supporting explicit TokenCredential, but I don't think this needs to be breaking, and would would not want to remove implicit DefaultAzureCredential usage anyway, which this change does. Can it be refactored so existing methods remain and forward to new overloads that accept TokenCredential? |
I will look at this next week. I agree that is also needed for more complex scenarios than this. |
…ensions.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ensions.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ons.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
As discussed, the change does actually make sense, even as a breaking change. Fine for it to be merged. |
Motivation and Context
This PR updates CosmosDB-related extension methods, allowing them to accept authentication token credentials instead of using a default one, which may not be deterministic enough for production scenarios.