-
Notifications
You must be signed in to change notification settings - Fork 686
.NET: Don't add OpenAIResponses as part of Dev UI #1977
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
You should be able to add and remove Dev UI without impacting your other production endpoints.
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 removes OpenAI Responses API support from the DevUI configuration, simplifying the DevUI to focus solely on Conversations functionality.
- Removes
AddOpenAIResponses()service registration from DevUI setup - Removes
MapOpenAIResponses()endpoint mapping from DevUI routing - Retains OpenAI Conversations API support for the DevUI
| var group = endpoints.MapGroup(""); | ||
| group.MapDevUI(pattern: "/devui"); | ||
| group.MapEntities(); | ||
| group.MapOpenAIConversations(); |
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.
Presumably, we should remove builder.Services.AddOpenAIConversations(); and group.MapOpenAIConversations(); too if we follow this logic; right?
Ideally MapDevUI would validate that its needed services and mappings were in place and throw an InvalidOperationException if not. But I think that could come later.
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.
Yeah, I think so. And agreed on providing a good error diagnostic.
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.
I think MapEntites() though is more Dev UI specific.
|
There are lots of DI-related APIs that add in necessary dependencies. Why is this different? |
@stephentoub With DI it's pretty easy to make the Add methods idempotent. Unfortunately, that's harder to do with Map methods in ASP.NET Core that add endpoints. |
| group.MapEntities(); | ||
| group.MapOpenAIConversations(); | ||
| group.MapOpenAIResponses(); | ||
| return group; |
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 be adding the bits we remove to dotnet\samples\GettingStarted\DevUI\DevUI_Step01_BasicUsage\Program.cs so that the sample still works
|
This should be closed in favor of (#2014) .NET: DevUI - Do not automatically add/map OpenAI services/endpoints |
You should be able to add and remove Dev UI without impacting your other production endpoints. This is the pattern used by Swagger UI, where you need to separate set up the OpenAPI endpoint that the Swagger UI depends on.