Skip to content

Commit

Permalink
mionr: use provider api keys presenter when server component loads cl…
Browse files Browse the repository at this point in the history
…ient component
  • Loading branch information
geclos committed Oct 2, 2024
1 parent 6adabbc commit 6fcd8a9
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
import { NAV_LINKS } from '$/app/(private)/_lib/constants'
import BreadcrumbLink from '$/components/BreadcrumbLink'
import { AppLayout } from '$/components/layouts'
import providerApiKeyPresenter from '$/presenters/providerApiKeyPresenter'
import { getCurrentUser } from '$/services/auth/getCurrentUser'
import { ROUTES } from '$/services/routes'

Expand Down Expand Up @@ -49,7 +50,7 @@ export default async function DocumentPage({
<EvaluationTabSelector evaluation={evaluation} />
<div className='flex-grow'>
<EvaluationEditor
providerApiKeys={providerApiKeys}
providerApiKeys={providerApiKeys.map(providerApiKeyPresenter)}

This comment has been minimized.

Copy link
@andresgutgon

andresgutgon Oct 2, 2024

Contributor

Why this is not done inside getProviderApiKeyCached? The consumer shouldn't be concerned with this. Is like api endpoints and presenters in old world

This comment has been minimized.

Copy link
@geclos

geclos Oct 2, 2024

Author Collaborator

yeah should probably be done there, i was just in a hurry here

evaluationUuid={evaluationUuid}
defaultPrompt={evaluation.metadata.prompt}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,12 @@ export default async function ConnectedEvaluationLayout({
metadata.config.provider &&
typeof metadata.config.provider === 'string'
) {
provider = await getProviderApiKeyCached(metadata.config.provider)
try {
provider = await getProviderApiKeyCached(metadata.config.provider)
} catch (error) {

This comment has been minimized.

Copy link
@andresgutgon

andresgutgon Oct 2, 2024

Contributor

But we should show error to the user no?

This comment has been minimized.

Copy link
@geclos

geclos Oct 2, 2024

Author Collaborator

no this is a valid flow not a real error that we should handle, it just so happens that our getter raises when it does not find what it's looking for (it should probably just return nil)

This comment has been minimized.

Copy link
@andresgutgon

andresgutgon Oct 2, 2024

Contributor

I don't understand. Why this is not an error worth it of showing to user?
For example if user enters on a document without commit what happens?

Image

This comment has been minimized.

This comment has been minimized.

Copy link
@geclos

geclos Oct 2, 2024

Author Collaborator

because it's a valid state to have no provider api key set up, which would result in this getter raising an error

This comment has been minimized.

Copy link
@andresgutgon

andresgutgon Oct 2, 2024

Contributor

Ah ok understood

// do nothing, it could be that the provider name does not match any
// provider in the workspace
}
}
}
return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
getDocumentsAtCommitCached,
getProviderApiKeysCached,
} from '$/app/(private)/_data-access'
import providerApiKeyPresenter from '$/presenters/providerApiKeyPresenter'

import DocumentEditor from './_components/DocumentEditor/Editor'

Expand All @@ -31,7 +32,7 @@ export default async function DocumentPage({
addMessagesAction={addMessagesAction}
documents={documents}
document={document}
providerApiKeys={providerApiKeys}
providerApiKeys={providerApiKeys.map(providerApiKeyPresenter)}
/>
)
}

0 comments on commit 6fcd8a9

Please sign in to comment.