-
Notifications
You must be signed in to change notification settings - Fork 15
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
Feature: Assistant Endpoints (Create, Modify, Delete, List) - Success #741
Feature: Assistant Endpoints (Create, Modify, Delete, List) - Success #741
Conversation
* support enum descriptions * added example --------- Co-authored-by: José Carlos Montañez <jc@MacBook-Pro.local> Co-authored-by: Raúl Raja Martínez <raulraja@gmail.com>
* Add README instructions for building Xef * Include reasons why build may fail if you don't have docker
Co-authored-by: José Carlos Montañez <jc@MacBook-Pro.local>
@@ -59,7 +59,7 @@ object Server { | |||
requestTimeout = 0 // disabled | |||
} | |||
install(Auth) | |||
install(Logging) { level = LogLevel.INFO } | |||
install(Logging) { level = LogLevel.ALL } |
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.
This is only needed for development, in main
this setting should be set to INFO
} | ||
} | ||
|
||
suspend fun createAssistant(request: CreateAssistantRequest): AssistantObject { |
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.
These functions are not needed. We can inline what they do on the call site. If you choose to keep them we can make them private
because they are only used in this file
</encoder> | ||
</appender> | ||
|
||
<root level="trace"> |
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.
The default level for logging in main
should be info
. We only use trace
while developing to see requests and responses when trying to troubleshoot a bug or issue.
@@ -0,0 +1,110 @@ | |||
// Este código asume que tienes un enum similar para los endpoints de la API de asistentes |
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.
We have the convention of using English as the primary language in the project, and we ask that any code comments be also in English. 🙏
defaultApiServer, | ||
} from '@/utils/api'; | ||
|
||
// Definiciones de tipos para las respuestas y solicitudes de asistentes (ajústalas a tu API) |
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.
// Definiciones de tipos para las respuestas y solicitudes de asistentes (ajústalas a tu API) | ||
export type AssistantRequest = { | ||
name: string; | ||
// Otros campos relevantes para la creación o actualización de un asistente |
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.
id: number; | ||
name: string; | ||
createdAt: string; | ||
// Otros campos que tu API devuelve |
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.
|
||
const assistantApiBaseOptions: ApiOptions = { | ||
endpointServer: defaultApiServer, | ||
endpointPath: EndpointsEnum.assistant, // Asegúrate de que este enum existe y es correcto |
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.
}, | ||
}; | ||
|
||
// POST: Crear un nuevo asistente |
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.
}; | ||
const apiConfig = apiConfigConstructor(apiOptions); | ||
const response = await apiFetch(apiConfig); | ||
return response.status; // Asumiendo que la API devuelve un código de estado para representar el resultado |
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.
@@ -1,14 +1,14 @@ | |||
import { | |||
/*import { |
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.
If these imports are not needed, we can remove them altogether
} | ||
}; | ||
|
||
const models = [ |
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.
Let's include the two identifiers for https://platform.openai.com/docs/models/gpt-4o since it was recently released, and we can make use of it here once we upgrade Xef to OpenAI v2 specs.
@@ -12,4 +12,5 @@ fun Routing.xefRoutes(logger: KLogger) { | |||
organizationRoutes(OrganizationRepositoryService(logger)) | |||
projectsRoutes(ProjectRepositoryService(logger)) | |||
tokensRoutes(TokenRepositoryService(logger)) | |||
assistantRoutes() |
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.
Let's add logging
to this route and use the logger in relevant parts of the code where needed to at least display calls to the endpoint.
For example:
logger.info("Creating assistant: ${assistant.name}")
...
logger.info("Created assistant: ${assistant.name} with id: ${assistant.id}")
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.
Great work, @mccarrascog and @JoseP3r32 👏 🎆 ! . I left a few comments that should be addressed before we merge this on the main. Please let me know if you need any clarification or if I can help you with any of the items.
… features/mc-assistant-form # Conflicts: # server/src/main/kotlin/com/xebia/functional/xef/server/http/routes/AssistantRoutes.kt
assistants.ts and Assistants.tsx - WIP |
This pull request introduces new endpoints for managing assistants:
These endpoints enable comprehensive management of assistants within our application.