-
Notifications
You must be signed in to change notification settings - Fork 300
fix: various fixes for openapi support #208
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
Conversation
var fragment struct { | ||
Paths map[string]any `json:"paths,omitempty"` |
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 want to check for the existence of the paths
map, rather than the version string, since the version string is sometimes missing.
func getOpenAPITools(t *openapi3.T, defaultHost string) ([]types.Tool, error) { | ||
// Determine the default server. | ||
if len(t.Servers) == 0 { | ||
return nil, fmt.Errorf("no servers found in OpenAPI spec") | ||
if defaultHost != "" { | ||
u, err := url.Parse(defaultHost) | ||
if err != nil { | ||
return nil, fmt.Errorf("invalid default host URL: %w", err) | ||
} | ||
u.Path = "/" | ||
t.Servers = []*openapi3.Server{{URL: u.String()}} | ||
} else { | ||
return nil, fmt.Errorf("no servers found in OpenAPI spec") | ||
} |
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 adds support for a "default host" if no server is specified at the global level in the OpenAPI document.
for _, param := range operation.Parameters { | ||
// Handle query, path, and header parameters, based on the parameters for this operation | ||
// and the parameters for this path. | ||
for _, param := range append(operation.Parameters, pathObj.Parameters...) { |
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.
Parameters can also be defined at the path level (pathObj.Parameters
), so we are now handling those as well.
if bodyMIME == "" { | ||
return nil, fmt.Errorf("no supported MIME types found for request body in operation %s", operation.OperationID) | ||
// No supported MIME types found, so just skip this operation and move on. | ||
continue operations |
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 now skip tools with no supported MIME types in the body instead of just returning an error for the whole document.
Signed-off-by: Grant Linville <[email protected]>
Signed-off-by: Grant Linville <[email protected]>
aa0c094
to
1a15239
Compare
These are fixes to various problems that Darren found with OpenAPI support. I annotated this PR to explain a bit about each substantial change.