Initial model card and schema routes #692
Conversation
| export interface ModelCardInterface { | ||
| schemaId: string | ||
| version: number | ||
| createdBy: string |
There was a problem hiding this comment.
Is this going to be just the username or is it worth adding in the full entity object?
There was a problem hiding this comment.
We should always use users whenever it's an action done by someone. There's never a 'group' that creates a release, it's always a specific person. (Also, entities are now strings of the form ${type}:${id}).
There was a problem hiding this comment.
Yes sorry I forgot the change in entity format, yeah I agree with this in that case.
|
|
||
| version: 1, | ||
| metadata: { example: true }, | ||
| console.log('CARD', card) |
There was a problem hiding this comment.
Was this added during development or do we need to log this
There was a problem hiding this comment.
My bad. I had some issues mocking and fell back to some console logging that I clearly forgot to remove. Nice catch!
|
|
||
| version: 1, | ||
| metadata: { example: true }, | ||
| if (!card) { |
There was a problem hiding this comment.
Why is this logic not in the service?
| } as const | ||
| export type GetModelCardVersionOptionsKeys = | ||
| (typeof GetModelCardVersionOptions)[keyof typeof GetModelCardVersionOptions] | ||
| export async function getModelCard(user: UserDoc, modelId: string, version: number | GetModelCardVersionOptionsKeys) { |
There was a problem hiding this comment.
I think we agreed not to use http methods in service names? Should this be f'indModelCardByModelId'?
| @@ -2,19 +2,16 @@ import bodyParser from 'body-parser' | |||
| import { Request, Response } from 'express' | |||
| import { z } from 'zod' | |||
|
|
|||
| import { ModelCardInterface } from '../../../../models/v2/ModelCard.js' | |||
| import { ModelCardInterface } from '../../../../models/v2/Model.js' | |||
| import { getModelCard as getModelCardService, GetModelCardVersionOptions } from '../../../../services/v2/model.js' | |||
There was a problem hiding this comment.
Why are you renaming this method? See previous comment about using http methods in service method names
This adds routes for: