Skip to content

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

Merged
merged 2 commits into from
Apr 2, 2024

Conversation

g-linville
Copy link
Member

@g-linville g-linville commented Apr 1, 2024

These are fixes to various problems that Darren found with OpenAPI support. I annotated this PR to explain a bit about each substantial change.

Comment on lines +328 to +329
var fragment struct {
Paths map[string]any `json:"paths,omitempty"`
Copy link
Member Author

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.

Comment on lines +19 to +31
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")
}
Copy link
Member Author

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...) {
Copy link
Member Author

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.

Comment on lines 152 to +177
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
Copy link
Member Author

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.

@g-linville g-linville marked this pull request as ready for review April 1, 2024 20:31
@g-linville g-linville merged commit 722acc3 into gptscript-ai:main Apr 2, 2024
@g-linville g-linville deleted the openapi-fixes branch April 2, 2024 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants