-
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
Gcp runtime #371
Gcp runtime #371
Conversation
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.
🚀 LGTM!
Just minor comments. Really good job! I have tested the code locally and everything works as expected
integrations/gcp/src/commonMain/kotlin/com/xebia/functional/xef/gcp/GcpConfig.kt
Show resolved
Hide resolved
integrations/gcp/src/commonMain/kotlin/com/xebia/functional/xef/gcp/GCP.kt
Outdated
Show resolved
Hide resolved
integrations/gcp/src/commonMain/kotlin/com/xebia/functional/xef/gcp/GCPChatScopeExtensions.kt
Outdated
Show resolved
Hide resolved
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.
🚀
examples/kotlin/src/main/kotlin/com/xebia/functional/xef/conversation/gpc/Chat.kt
Outdated
Show resolved
Hide resolved
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.
Looks great!, thank you, @Intex32 . I think before merging these, we should address the concern around the default parameters in the GCP extensions.
integrations/gcp/src/commonMain/kotlin/com/xebia/functional/xef/gcp/GCPChatScopeExtensions.kt
Outdated
Show resolved
Hide resolved
openai/src/commonMain/kotlin/com/xebia/functional/xef/conversation/llm/openai/OpenAI.kt
Outdated
Show resolved
Hide resolved
We cannot provide defaults as we need access to the GCP instance (which is not accessible through the conversion). In contrast to OpenAI we have additional parameters "location" and "projectId". |
integrations/gcp/src/commonMain/kotlin/com/xebia/functional/xef/gcp/GcpScopeExtensions.kt
Outdated
Show resolved
Hide resolved
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.
Everything looks great, thanks @Intex32 !,
can we rename the functions to not include the Gcp
suffix in https://github.com/xebia-functional/xef/pull/371/files#r1312844834 ?
examples/kotlin/src/main/kotlin/com/xebia/functional/xef/conversation/gpc/Chat.kt
Outdated
Show resolved
Hide resolved
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.
🚀 LGTM!
rework of GCP and Conversation for the 0.0.3 release