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

Add ApiKey to TEI embedding provider #4783

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

YohannZe
Copy link
Contributor

Description

[ What changed? Feel free to be brief. ]

Added the possibility to have an api-key in the tei embedding provider, following what was done for the rerank endpoint.

Checklist

  • The relevant docs, if any, have been updated or created
  • The relevant tests, if any, have been updated or created

Screenshots

[ For visual changes, include screenshots. ]

Testing instructions

[ For new or modified features, provide step-by-step testing instructions to validate the intended behavior of the change, including any relevant tests to run. ]

@YohannZe YohannZe requested a review from a team as a code owner March 24, 2025 12:42
@YohannZe YohannZe requested review from sestinj and removed request for a team March 24, 2025 12:42
Copy link

netlify bot commented Mar 24, 2025

Deploy Preview for continuedev ready!

Name Link
🔨 Latest commit 9ee7190
🔍 Latest deploy log https://app.netlify.com/sites/continuedev/deploys/67e1533d71eda60008686264
😎 Deploy Preview https://deploy-preview-4783--continuedev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Collaborator

@RomneyDa RomneyDa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@YohannZe I believe this was fixed 2 weeks ago around the same time as your PR was opened on #4581. Sorry for the review delay.

apiKey is added to headers in the this.fetch function that uses our custom @continuedev/fetch package to apply request options to the request. Huggingface was using fetch instead of this.fetch. Your approach would also fix the api key but other request options would still be ignored.

Could you verify that this has been fixed on your end?

@YohannZe
Copy link
Contributor Author

Hello @RomneyDa , thanks for the reply !
On my side, I don't see any changes with before, and no api key in the request made to the embedding provider. I tried both pre release and release version.

I've read your PR, could it be that in it HuggingFaceTei extends BaseLLM where it should extend openAI ? I'm not familiar with the code so I could be wrong.

Thanks for the review, I remain available if anything.

@RomneyDa
Copy link
Collaborator

RomneyDa commented Mar 31, 2025

OpenAI extends BaseLLM and this.fetch is set up on the underlying BaseLLM. If you can debug and step through/into this.fetch you might find the issue.

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.

2 participants