Skip to content
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

Merged
merged 20 commits into from
May 23, 2024

Conversation

mccarrascog
Copy link
Collaborator

This pull request introduces new endpoints for managing assistants:

  • Create Assistant: Adds functionality to create new assistants.
  • Modify Assistant: Implements the capability to modify existing assistants.
  • Delete Assistant: Includes deletion functionality for assistants.
  • List Assistants: Provides a feature to retrieve a list of all available assistants.

These endpoints enable comprehensive management of assistants within our application.

JoseP3r32 and others added 13 commits April 23, 2024 15:33
* 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>
@mccarrascog mccarrascog requested a review from raulraja May 15, 2024 14:58
@@ -59,7 +59,7 @@ object Server {
requestTimeout = 0 // disabled
}
install(Auth)
install(Logging) { level = LogLevel.INFO }
install(Logging) { level = LogLevel.ALL }
Copy link
Contributor

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 {
Copy link
Contributor

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">
Copy link
Contributor

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
Copy link
Contributor

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)
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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 {
Copy link
Contributor

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 = [
Copy link
Contributor

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()
Copy link
Contributor

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}")

Copy link
Contributor

@raulraja raulraja left a 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.

@mccarrascog mccarrascog changed the base branch from main to features/assistant-create-form May 16, 2024 13:55
@mccarrascog mccarrascog merged commit fb7c1ca into features/assistant-create-form May 23, 2024
@mccarrascog mccarrascog deleted the features/mc-assistant-form branch May 23, 2024 08:24
@mccarrascog
Copy link
Collaborator Author

assistants.ts and Assistants.tsx - WIP
Added icons in the sidebar
All other files - OK, with all corrections made.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants