feat: add credential management #65
Conversation
Signed-off-by: Grant Linville <[email protected]>
| case []any: | ||
| b, err := json.Marshal(out) | ||
| if err != nil { | ||
| r.state = Error | ||
| r.err = fmt.Errorf("failed to process stdout: %w", err) | ||
| return | ||
| } | ||
|
|
||
| r.output = string(b) |
There was a problem hiding this comment.
Is this correct? Maybe I should just return a string in the SDK instead of the full slice?
Signed-off-by: Grant Linville <[email protected]>
g-linville
requested review from
thedadams,
StrongMonkey,
iwilltry42 and
tylerslaton
| @@ -1448,3 +1451,37 @@ func TestLoadTools(t *testing.T) { | |||
| t.Errorf("Unexpected name: %s", prg.Name) | |||
| } | |||
| } | |||
|
|
|||
| func TestCredentials(t *testing.T) { | |||
| @@ -325,6 +325,15 @@ func (r *Run) request(ctx context.Context, payload any) (err error) { | |||
|
|
|||
| done, _ = out["done"].(bool) | |||
| r.rawOutput = out | |||
| case []any: | |||
There was a problem hiding this comment.
Is this checking if credential returns an array of objects? if so then it seems fine to me
|
I would like to get gptscript-ai/gptscript#849 merged prior to this one. Then I can come back and update this to properly support the changes from that PR. |
Signed-off-by: Grant Linville <[email protected]>
|
This one is updated and ready again. |
| require github.com/getkin/kin-openapi v0.124.0 | ||
| require ( | ||
| github.com/getkin/kin-openapi v0.124.0 | ||
| github.com/stretchr/testify v1.8.4 |
There was a problem hiding this comment.
nit: I would prefer to not have this.
There was a problem hiding this comment.
nit: why? doesn't everyone use testify? What else would you use?
gptscript.go
Outdated
| @@ -307,6 +307,60 @@ func (g *GPTScript) PromptResponse(ctx context.Context, resp PromptResponse) err | |||
| return err | |||
| } | |||
|
|
|||
| func (g *GPTScript) ListCredentials(ctx context.Context, credCtxs []string, allContexts bool) ([]Credential, error) { | |||
There was a problem hiding this comment.
Can credCtxs and allContext be added to an Options struct. If no options are passed is should do the same thing as gptscript creds which I really don't know if that is "all contexts" or what.
Signed-off-by: Grant Linville <[email protected]>
gptscript.go
Outdated
| Name: name, | ||
| }) | ||
| if err != nil { | ||
| if code == 404 { |
There was a problem hiding this comment.
nit:
| if code == 404 { | |
| if code == http.StatusNotFound { |
Signed-off-by: Grant Linville <[email protected]>
Signed-off-by: Grant Linville <[email protected]>
Signed-off-by: Grant Linville <[email protected]>
Signed-off-by: Grant Linville <[email protected]>
Signed-off-by: Grant Linville <[email protected]>
Signed-off-by: Grant Linville <[email protected]>
Signed-off-by: Grant Linville <[email protected]>
opts.go
Outdated
| @@ -66,6 +66,7 @@ type Options struct { | |||
| IncludeEvents bool `json:"includeEvents"` | |||
| Prompt bool `json:"prompt"` | |||
| CredentialOverrides []string `json:"credentialOverrides"` | |||
| CredentialContext []string `json:"credentialContext"` | |||
There was a problem hiding this comment.
Let's make the field plural, but keep the json tag the same so that it is compatible with the SDK server. Maybe add a comment that this difference is intentional.
Also, it is possible to add a test for this?
Signed-off-by: Grant Linville <[email protected]>
Signed-off-by: Grant Linville <[email protected]>
Signed-off-by: Grant Linville <[email protected]>
gptscript.go
Outdated
| credCtxs []string | ||
| allContexts bool |
There was a problem hiding this comment.
These need to be updated.
Signed-off-by: Grant Linville <[email protected]>
depends on gptscript-ai/gptscript#846