-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat(api)!: support extra_body to embeddings and vector_stores APIs #3794
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
Conversation
…s APIs Applies the same pattern from #3777 to embeddings and vector_stores.create() endpoints. Breaking change: Method signatures now accept a single params object with Pydantic extra="allow" instead of individual parameters. Provider-specific params can be passed via extra_body and accessed through params.model_extra. Updated APIs: openai_embeddings(), openai_create_vector_store(), openai_create_vector_store_file_batch()
VectorIORouter was still using old individual parameter signature instead of the new params object. Updated both openai_create_vector_store and openai_create_vector_store_file_batch methods to match the API protocol.
…thods VectorDBsRoutingTable was removed in a165b8b, so VectorIORouter needs to get the provider directly using routing_table.get_provider_impl() before calling provider methods, consistent with how insert_chunks() already works.
# Extract llama-stack-specific parameters from extra_body | ||
extra = params.model_extra or {} | ||
embedding_model = extra.get("embedding_model") | ||
embedding_dimension = extra.get("embedding_dimension", 384) |
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.
do we still want this default?
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 guess so for tests
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.
@franciscojavierarceo yeah I should kill that and see what breaks
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
Green finally. Corresponding llama-stack-client changes: llamastack/llama-stack-client-python#280 |
…lamastack#3794) Applies the same pattern from llamastack#3777 to embeddings and vector_stores.create() endpoints. This should _not_ be a breaking change since (a) our tests were already using the `extra_body` parameter when passing in to the backend (b) but the backend probably wasn't extracting the parameters correctly. This PR will fix that. Updated APIs: `openai_embeddings(), openai_create_vector_store(), openai_create_vector_store_file_batch()`
Applies the same pattern from #3777 to embeddings and vector_stores.create() endpoints.
This should not be a breaking change since (a) our tests were already using the
extra_body
parameter when passing in to the backend (b) but the backend probably wasn't extracting the parameters correctly. This PR will fix that.Updated APIs:
openai_embeddings(), openai_create_vector_store(), openai_create_vector_store_file_batch()