feat: sdkserver: add credential routes #846
Conversation
Signed-off-by: Grant Linville <[email protected]>
| mux.HandleFunc("POST /credentials", s.listCredentials) | ||
| mux.HandleFunc("POST /credentials/create", s.createCredential) | ||
| mux.HandleFunc("POST /credentials/reveal", s.revealCredential) |
There was a problem hiding this comment.
Let me know if these routes should change at all or if this is fine. Feels kinda weird to have it this way, but it's also abnormal to do a request body with a GET, so...I just did this.
There was a problem hiding this comment.
The alternative is to use path and query parameters (e.g. /credentials/reveal/name?context=my-context or /credentials/delete/name?context=my-context (and make this DELETE)).
I leave the ultimate decision up to you. We certainly aren't building a REST API. This is more of RPC stuff, which I think is almost always POST.
I'll leave a final review after your response.
There was a problem hiding this comment.
Cool, I think I will leave it as it is then. All POST is what works best with the way the Go SDK is set up, at least.
| // TODO - are we sure we want to always use runtimes.Default here? | ||
| store, err := credentials.NewStore(cfg, runtimes.Default(s.gptscriptOpts.Cache.CacheDir), ctx, s.gptscriptOpts.Cache.CacheDir) |
There was a problem hiding this comment.
Right now, runtimes.Default is the only RuntimeManager in GPTScript, as far as I am aware. I would initialize this the normal way through s.gptscriptOpts, but the Runner opts there are nil, so I just did this instead.
g-linville
requested review from
thedadams,
StrongMonkey,
iwilltry42 and
tylerslaton
Signed-off-by: Grant Linville <[email protected]>
I've tested all of these with curl and they work properly.