-
Notifications
You must be signed in to change notification settings - Fork 632
feat(genkit-tools): add /listValues endpoint to expose defaultModel
#4044
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -80,6 +80,22 @@ export const ListActionsRequestSchema = z | |
|
|
||
| export type ListActionsRequest = z.infer<typeof ListActionsRequestSchema>; | ||
|
|
||
| export const ListValuesRequestSchema = z.object({ | ||
| runtimeId: z | ||
| .string() | ||
| .optional() | ||
| .describe( | ||
| 'ID of the Genkit runtime to run the action on. Typically $pid-$port.' | ||
| ), | ||
| type: z | ||
| .enum(['defaultModel']) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Restricted to |
||
| .describe( | ||
| "The type of values to fetch. Currently only supports 'defaultModel'" | ||
| ), | ||
| }); | ||
|
|
||
| export type ListValuesRequest = z.infer<typeof ListValuesRequestSchema>; | ||
|
|
||
| export const RunActionRequestSchema = z.object({ | ||
| runtimeId: z | ||
| .string() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,6 +46,7 @@ import { | |
| type GenerationCommonConfigSchema, | ||
| type IndexerParams, | ||
| type ModelArgument, | ||
| type ModelReference, | ||
| type Part, | ||
| type PromptConfig, | ||
| type PromptGenerateOptions, | ||
|
|
@@ -953,7 +954,7 @@ export class Genkit implements HasRegistry { | |
| this.registry.registerValue( | ||
| 'defaultModel', | ||
| 'defaultModel', | ||
| this.options.model | ||
| modelName(this.options.model) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Realized that having the conversion done at registry time is more convenient, since all types needed are available here. WDYT? |
||
| ); | ||
| } | ||
| if (this.options.promptDir !== null) { | ||
|
|
@@ -1086,3 +1087,19 @@ let disableReflectionApi = false; | |
| export function __disableReflectionApi() { | ||
| disableReflectionApi = true; | ||
| } | ||
|
|
||
| /** Helper method to map ModelArgument to string */ | ||
| function modelName( | ||
| modelArg: ModelArgument<any> | undefined | ||
| ): string | undefined { | ||
| if (modelArg === undefined) { | ||
| return undefined; | ||
| } | ||
| if (typeof modelArg === 'string') { | ||
| return modelArg; | ||
| } | ||
| if ((modelArg as ModelReference<any>).name) { | ||
| return (modelArg as ModelReference<any>).name; | ||
| } | ||
| return (modelArg as ModelAction).__action.name; | ||
| } | ||
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.
backwards compatibility consideration -- Go and python won't have this endpoint implemented, so will be getting 404. Should 404 be handled here and an empty record returned, or bubble up the error all the way to the Dev UI and let it deal with backwards compatibility?
I'm leaning towards handle 404 here...