-
Notifications
You must be signed in to change notification settings - Fork 589
.NET: dotnet samples suggestions #1581
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?
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 .NET sample code to improve consistency and simplify named arguments. The key changes remove redundant named arguments for cancellationToken parameters, replace generic Exception types with more appropriate exception types in some cases, and adjust the CA2201 diagnostic severity to allow generic exceptions in samples.
Key Changes:
- Removed unnecessary
cancellationToken:named arguments throughout workflow samples, simplifying method calls - Changed exception types from
InvalidOperationExceptiontoExceptionorArgumentExceptionwhere appropriate - Updated
.editorconfigto change CA2201 diagnostic from warning to suggestion, allowing generic exceptions in sample code
Reviewed Changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| dotnet/.editorconfig | Changed CA2201 severity from warning to suggestion to allow generic exceptions |
| dotnet/samples/GettingStarted/Workflows/_Foundational/04_AgentWorkflowPatterns/Program.cs | Changed exception type to Exception and removed trailing period from comment |
| dotnet/samples/GettingStarted/Workflows/_Foundational/03_AgentsInWorkflows/Program.cs | Changed exception type from InvalidOperationException to Exception |
| dotnet/samples/GettingStarted/Workflows/Loop/Program.cs | Removed named cancellationToken: arguments from SendMessageAsync calls |
| dotnet/samples/GettingStarted/Workflows/HumanInTheLoop/HumanInTheLoopBasic/WorkflowHelper.cs | Removed named cancellationToken: arguments from SendMessageAsync calls |
| dotnet/samples/GettingStarted/Workflows/HumanInTheLoop/HumanInTheLoopBasic/Program.cs | Refactored HandleExternalRequest to use nullable switch pattern and ArgumentException |
| dotnet/samples/GettingStarted/Workflows/ConditionalEdges/03_MultiSelection/Program.cs | Changed exception types and removed named cancellationToken arguments |
| dotnet/samples/GettingStarted/Workflows/ConditionalEdges/02_SwitchCase/Program.cs | Changed exception types from InvalidOperationException to Exception/ArgumentException |
| dotnet/samples/GettingStarted/Workflows/ConditionalEdges/01_EdgeCondition/Program.cs | Changed exception types from InvalidOperationException to Exception/ArgumentException |
| dotnet/samples/GettingStarted/Workflows/Concurrent/MapReduce/Program.cs | Removed named cancellationToken: arguments from SendMessageAsync calls |
| dotnet/samples/GettingStarted/Workflows/Concurrent/Concurrent/Program.cs | Changed exception type and removed named cancellationToken arguments |
| dotnet/samples/GettingStarted/Workflows/Checkpoint/CheckpointWithHumanInTheLoop/WorkflowHelper.cs | Removed named cancellationToken: arguments from multiple methods |
| dotnet/samples/GettingStarted/Workflows/Checkpoint/CheckpointAndResume/WorkflowHelper.cs | Removed named cancellationToken: arguments from multiple methods |
| dotnet/samples/GettingStarted/Workflows/Checkpoint/CheckpointAndRehydrate/WorkflowHelper.cs | Removed named cancellationToken: arguments from multiple methods |
| dotnet/samples/GettingStarted/Workflows/Agents/WorkflowAsAnAgent/WorkflowHelper.cs | Removed named cancellationToken: arguments from SendMessageAsync calls |
| dotnet/samples/GettingStarted/Workflows/Agents/WorkflowAsAnAgent/Program.cs | Changed exception type from InvalidOperationException to Exception |
| dotnet/samples/GettingStarted/Workflows/Agents/FoundryAgent/Program.cs | Changed exception type and updated comment from OpenAI to Foundry |
| dotnet/samples/GettingStarted/Workflows/Agents/CustomAgentExecutors/Program.cs | Changed exception type and removed named cancellationToken argument |
| dotnet/samples/AgentWebChat/AgentWebChat.Web/OpenAIResponsesAgentClient.cs | Removed named cancellationToken: argument from GetStreamingResponseAsync call |
dotnet/samples/GettingStarted/Workflows/HumanInTheLoop/HumanInTheLoopBasic/Program.cs
Show resolved
Hide resolved
| { | ||
| // Set up the Azure OpenAI client | ||
| var endpoint = Environment.GetEnvironmentVariable("AZURE_OPENAI_ENDPOINT") ?? throw new InvalidOperationException("AZURE_OPENAI_ENDPOINT is not set."); | ||
| var endpoint = Environment.GetEnvironmentVariable("AZURE_OPENAI_ENDPOINT") ?? throw new Exception("AZURE_OPENAI_ENDPOINT is not set."); |
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.
What is the reason to use a less concrete exception?
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.
The change is about not throwing InvalidOperationException since those are meant for instance method calls and convey that the object's state is invalid.
https://learn.microsoft.com/dotnet/api/system.invalidoperationexception
The exception that is thrown when a method call is invalid for the object's current state.
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.
Then perhaps another exception type? For example: "ArgumentException".
@TaoChenOSU @westey-m