-
Notifications
You must be signed in to change notification settings - Fork 241
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
Overlay support spec doc #1598
base: dev
Are you sure you want to change the base?
Overlay support spec doc #1598
Conversation
Add spec for overlay support
Add a folder for specs
Specs/Overlay Support Spec.md
Outdated
Support use of Overlays for enabling developers to enhance existing OpenAPI description files without changing the original file. | ||
|
||
## Problem Statement | ||
Existing OpenAPI documents used for AI plugin creation might lack necessary properties or require modifications for them to provide a high quality AI plugin. Direct editing of the original OpenAPI document is often undesirable or impractical. |
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.
Not AI plugin, but I'd even say referenced OpenAPI descriptions that participate in the GPTs and Plugins ecosystem.
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, you have phrased it better. I'll rephrase in the spec to capture this.
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.
Why restrict the use-case of overlays to AI plug-ins when OpenAPI.NET library caters to a diverse range of users?
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.
@adhiambovivian has a good point. What sparked this discussion was definitely AI plugins. But it's absolutely true that overlays are more than just for AI plugins, we see it everyday in Kiota. Let's frame it as a more generic scenario and support it with real world examples, like AI plugins, client code generation, etc.
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.
And for the copilot use-cases, could you please provide more specific details about the copilot use-cases?
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.
Sure @adhiambovivian . For copilot scenarios we might would want to provide instructions to the orchestrator on how to use the certain function (/createSong as an exmaple), add localization, provide information on how to present the response to the user (like using adaptive cards or other rendering) etc. Having the possibility to augment OpenAPI documents with overlays and provide more information to AI orchestratos/Copilot will unclock several scenarios.
Specs/Overlay Support Spec.md
Outdated
description: Successful response | ||
``` | ||
|
||
**Overlay File (overlay.yaml)** |
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.
This overlay doesn't use JSON Path to define the structure. Are we saying that this is our MVP for now? Only structured overlays vs. targeted overlays would be supported?
https://github.com/OAI/Overlay-Specification/blob/main/versions/1.0.0.md#targeted-overlays
I'm honestly OK at this point for either, but make it clear would be better and provide a more appropriate debate!
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.
You can't really separate the two. Targeted overlays use a merge patch at the target. Structured overlays are just the trivial case of targeted.
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.
@sebastienlevert is your question whether we will support either structured or targeted rather than both? We will support both cases.
We should add some discussion of the developer experience of applying an overlay. Is this a separate OverlayService class? Is it an extension method on OpenAPIDocument? Does it change the existing document or create a new patched document? Is it in a different project? If not, how do we handle the YAML dependency? |
@@ -0,0 +1,106 @@ | |||
# Feature Specification: Overlays Support | |||
|
|||
## Objective |
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.
Shouldn't the spec include customer application and target audience?
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 have revised the objective statement to try capture the target audience.
Maybe use this issue to add onto some of the use cases or scenarios where overlays would come in handy #615 |
expand the problem statement to capture more areas of challenges.
@darrelmiller I have added a section on 'User Journey' to start the discussion. Have a look at it and continue the discussion. |
revised with reviewers' comments
Specs/Overlay Support Spec.md
Outdated
3. Use the OverlayService to apply the overlay file to the OpenAPI document. The OverlayService parses the overlay file, applies the changes directly to the OpenApiDocument instance, and performs in-place modifications. | ||
```csharp | ||
var overlayService = new OverlayService(); | ||
overlayService.ApplyOverlay(openApiDocument, overlayPath); |
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.
overlayService.ApplyOverlay(openApiDocument, overlayPath); | |
var overlay = await Overlay.LoadAsync(stream); | |
var jsonElement = await JsonDocument.ParseAsync(stream).RootElement; | |
var newJsonElement = overlay.Apply(jsonElement); |
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.
End to end process would be:
- Use OpenApiDocument.Load to load JSON/Yaml file with Overlay referenced in readerSettings
- Load Yaml and translate to JSONNodes or just load JSONNodes
- Load Overlay as JsonNodes.
- Apply Overlay to OpenAPI JsonNodes
- Pass "overlayed" JsonNodes to rest of OpenAPI Parser.
address reviewer comments. Add example for targeted overlay file.
add a folder for specs and add the first spec doc for overlay support.