-
Notifications
You must be signed in to change notification settings - Fork 137
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
[WIP] (feat) Add API tool (from bee-api) #132
Conversation
Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
Signed-off-by: Nigel Jones <jonesn@uk.ibm.com>
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.
Rename class to OpenAPI
and place it in src/tools/openapi.ts
.
import { SchemaObject } from 'ajv'; | ||
import { parse } from 'yaml'; | ||
import { isEmpty } from 'remeda'; | ||
import axios from 'axios'; |
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.
Don't use axios
, use standard fetch
.
|
||
import { BaseToolOptions, StringToolOutput, Tool, ToolError } from '@/tools/base.js'; | ||
import { SchemaObject } from 'ajv'; | ||
import { parse } from '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.
yaml
package must be added to peerDependencies
import { HttpProxyAgent } from 'http-proxy-agent'; | ||
import { HttpsProxyAgent } from 'https-proxy-agent'; |
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.
both packages must be added to peerDependencies
name: string; | ||
description: string; | ||
openApiSchema: any; | ||
apiKey?: string; |
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.
make protected
url: url.toString(), | ||
data: !isEmpty(input.body) ? input.body : undefined, | ||
method: input.method.toLowerCase(), | ||
headers, |
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.
use headers directly here
} | ||
} | ||
|
||
protected async _run(input: any) { |
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 would try to use protected async _run(input: SchemaObject) {
instead of the any
type.
if (!this.openApiSchema?.paths) { | ||
throw new ToolError("Server is not specified!") | ||
} |
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.
Use ValueError
instead of ToolError
in constructor.
if (this.apiKey) { | ||
//TODO: #105 confirm ok to remove use of encrypted api key | ||
headers['Authorization'] = `Bearer ${this.apiKey}`; | ||
} |
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 would consider refactoring the whole URL building part by leveraging createURLParams
function from src/internals/fetcher.ts
module.
}); | ||
return new StringToolOutput(response.data); | ||
} catch (error) { | ||
throw new ToolError(`Request to ${url} failed.`); |
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.
Update to
throw new ToolError(`Request to ${url} failed.`, [error]);
Hello, @planetf1. Are you willing to continue on this? |
Closing for inactivity. |
Which issue(s) does this pull-request address?
Ref: #105
Description
Implements support for calling an API. Originally part of bee-api, this has now been moved into the framework as a tool.
An openAPI spec of the endpoint is required
Test support is included
NOTE This is still work in progress - opening up as a draft. Feedback welcome. Would like to check if this is the kind of thing you are looking for.
Lint/formatting checks are needed, along with additional tests within the yarn test:e2e framework, as well as validation the tool can be used more generally with bee.
I also need to identify a 'proper' rest endpoint to test against - either external, or by implementing a simple server within the test framework.
There are also TODOs and commented lines still left in the code. These will be resolved before moving PR out of draft
Checklist
yarn lint
oryarn lint:fix
yarn format
oryarn format:fix
yarn test:unit
yarn test:e2e