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

Fix /vertex payload parsing when MESSAGES_API_ENABLED #2552

Closed
wants to merge 2 commits into from

Conversation

alvarobartt
Copy link
Member

What does this PR do?

This PR fixes the input payload parsing on Vertex AI i.e. when compiled with the google feature and deployed in a Vertex AI environment (replicable with the following environment variables AIP_MODE=PREDICTION, AIP_HTTP_PORT=80, AIP_PREDICT_ROUTE=/predict, and AIP_HEALTH_ROUTE=/health); when the environment variable MESSAGES_API_ENABLED is set to true, meaning that the /vertex endpoint is a proxy for /v1/chat/completions endpoint instead of the default /generate.

Note

As already discussed with @Narsil this is most likely not the best approach, here are some things that I tried:

  • Implementing the From trait to go from the input Vertex AI payload into ChatRequest, but that still requires us to have a "duplicated" struct holding the generation kwargs, which is not ideal
  • The above could be solved by moving the generation kwargs out of ChatRequest whilst just leaving messages there, and using #[serde(flatten)] over the generation kwargs; but that's not ideal either
    Any feedback is appreciated!

Who can review?

@Narsil as of our previous conversation today, feel free to take over the PR, or even close it in favour of a potentially better PR! Thanks in advance Nicolas! 🤗

@alvarobartt alvarobartt marked this pull request as draft September 23, 2024 18:58
@alvarobartt alvarobartt marked this pull request as ready for review September 23, 2024 19:24
@alvarobartt
Copy link
Member Author

Closed in favour of #2553, thank you @Narsil!

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.

1 participant