Initial Pulze Provider #153
Conversation
|
Nico Kupfer seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
customize json unmarshalling to support old settings structure
| @@ -169,10 +179,9 @@ func (a *App) CheckHealth(ctx context.Context, req *backend.CheckHealthRequest) | |||
| a.healthCheckMutex.Lock() | |||
| defer a.healthCheckMutex.Unlock() | |||
|
|
|||
There was a problem hiding this comment.
nit: I'm thinking we can remove the error returned from this function as well. we aren't returning an error anywhere.
There was a problem hiding this comment.
I thought so too, but at the moment it's required to ensure App complies to the CheckHealthHandler interface (https://github.com/grafana/grafana-plugin-sdk-go/blob/main/backend/diagnostics.go#L12).
pkg/plugin/health.go
Outdated
| @@ -11,7 +11,11 @@ import ( | |||
| "github.com/grafana/grafana-plugin-sdk-go/build" | |||
| ) | |||
|
|
|||
| var openAIModels = []string{"gpt-3.5-turbo", "gpt-4"} | |||
| // Define models for each provider to be included in the health check. | |||
| var providerModels = map[string][]string{ | |||
There was a problem hiding this comment.
i think this would be more type-safe if it was a map[openAIProvider][]string
| if a.healthOpenAI != nil { | ||
| return *a.healthOpenAI, nil | ||
| return *a.healthOpenAI | ||
| } | ||
|
|
||
| d := openAIHealthDetails{ | ||
| OK: true, | ||
| Configured: a.settings.OpenAI.apiKey != "" || a.settings.OpenAI.Provider == openAIProviderGrafana, | ||
| Models: map[string]openAIModelHealth{}, |
There was a problem hiding this comment.
Why are we removing the condition for openAIProviderGrafana here?
There was a problem hiding this comment.
I've changed the logic here (e.g. all "providers" are listed in providerModels now), hence the lookup can use a.settings.OpenAI.Provider.
|
|
||
| openAIKey = "openAIKey" | ||
| llmGatewayKey = "llmGatewayKey" | ||
| encodedTenantAndTokenKey = "base64EncodedAccessToken" | ||
| ) | ||
|
|
||
| // OpenAISettings contains the user-specified OpenAI connection details |
There was a problem hiding this comment.
I think at some point we'll have to consider renaming this to ProviderSettings or something.. (but can be done in a future PR)
There was a problem hiding this comment.
Yep, agree. We initially planned to include this, but it's better to have a separate PR for this 👍
|
Thanks for your review, I've updated the PR. Let me know if there's anything else |
|
Sorry for the delay on reviewing this, we're thinking about some redesigns to make things easier for users and plugin developers but we haven't forgotten! |
Cool, no worries. We'll keep this PR up to date. Let us know if there's anything we can help you with. |
|
Hey @fabianbaier, forgot to follow up on this one. |
Draft of adding Pulze.ai as provider