-
-
Notifications
You must be signed in to change notification settings - Fork 62
[Platform] Add support for Google vertex AI #297
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
[Platform] Add support for Google vertex AI #297
Conversation
14ae923
to
35d9a8b
Compare
35d9a8b
to
9124195
Compare
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, ship it
9124195
to
cd83b3b
Compare
Docs and AI bundle integration need to be added first. |
1346440
to
b08d7e7
Compare
* } | ||
* } |
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 indention looks wrong
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.
In the PHP doc, the IDE doesn't apparently help with the indentation in this case.
I tried to correct the indentation now.
153ee1e
to
ce696af
Compare
Hmm, super weird 🤔 And where is that |
@chr-hertel
P.S. I made a change now to remove the location from the start of the request URL for both text and embedding generation after verifying it works. It matches the URL that you referenced, but the location is still there in the middle. |
and also with that endpoint change the examples
Btw, code-wise we're good here, thanks for that! I just want to be able to run all examples locally to make sure all integrations are still working. And I'm not super deep into google cloud. is there something special that you configured in vertex? global region? any models or endpoints in action here? |
b9872b7
to
6f392c8
Compare
@chr-hertel Changes I made:Vertex AI:
Locally:
|
@chr-hertel |
Code-wise all looks good to me - mostly depends on me to find the issue of testing. Not gonna happen this week tho, sorry. |
{ | ||
$metadata = $output->result->getMetadata(); | ||
|
||
if ($output->result instanceof StreamResult) { |
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.
Cf #347 (comment) I'm dunno if StreamResult should be handled here...
Maybe it's better to have the same behavior everywhere // Streams have to be handled manually as the tokens are part of the streamed chunks
and to open an issue first to discuss about it ?
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.
I left a comment there.
@chr-hertel |
cfceb9d
to
bacfee6
Compare
@chr-hertel |
@chr-hertel is currently AFK for some days. |
Oh ok. |
one thing i learned lately, that it fails due to the region - that was a good pointer of yours - so you could reproduce with switching to |
f106bf0
to
fd9f19b
Compare
@chr-hertel |
1c5b63e
to
df694ad
Compare
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.
Thanks for your patience and dedication here - let's get this merged now! 👍
df694ad
to
b1f5046
Compare
Thank you @junaidbinfarooq. |
Changes proposed: