-
Notifications
You must be signed in to change notification settings - Fork 266
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
feat: add dataset operations to sdkserver #869
Conversation
Signed-off-by: Grant Linville <[email protected]>
Signed-off-by: Grant Linville <[email protected]>
Signed-off-by: Grant Linville <[email protected]>
pkg/gptscript/gptscript.go
Outdated
if result.DatasetToolRepo == "" { | ||
result.DatasetToolRepo = defaultDatasetToolRepo | ||
} |
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 don't think this is needed given that you have a default set.
I do believe it is needed here, though, when running the SDK server in embedded mode:
gptscript/pkg/sdkserver/server.go
Line 154 in 45d444f
func complete(opts ...Options) Options { |
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.
Thanks. This is fixed
Signed-off-by: Grant Linville <[email protected]>
Signed-off-by: Grant Linville <[email protected]>
Signed-off-by: Grant Linville <[email protected]>
pkg/sdkserver/datasets.go
Outdated
Name string `json:"dataset_name"` | ||
Description string `json:"dataset_description"` |
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.
nit: you have camelCase and snake_case in your json
tags. Can this be standardized?
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.
fixed
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 went the opposite way I expected. Is there reason why you are doing snake_case? I think most of our stuff uses camelCase.
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.
Lol. I honestly have no idea. I'll go change it all to camelCase.
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.
Fixed. Sorry for the delay. Got stuck working on a test for another repo
Signed-off-by: Grant Linville <[email protected]>
Signed-off-by: Grant Linville <[email protected]>
Signed-off-by: Grant Linville <[email protected]>
This adds five new operations to the SDKServer to run the tools that we are using as the API for datasets. The SDKServer is basically just acting as a middleman between the client using the SDK, and the dataset API (the five tools in gptscript-ai/datasets#1).