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

Allow specifying cache dir for fastembed #360

Closed
wants to merge 1 commit into from

Conversation

KaQuMiQ
Copy link

@KaQuMiQ KaQuMiQ commented Oct 30, 2023

This commit adds ability to specify cache dir for fastembed when setting up the client embedding model. This allows managing models data locally and ensure proper/altered paths on docker when needed.

All Submissions:

  • Contributions should target the dev branch. Did you create your branch from dev?
  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

New Feature Submissions:

  1. Does your submission pass tests?
  2. Have you installed pre-commit with pip3 install pre-commit and set up hooks with pre-commit install?

@netlify
Copy link

netlify bot commented Oct 30, 2023

Deploy Preview for poetic-froyo-8baba7 ready!

Name Link
🔨 Latest commit 9ec99dc
🔍 Latest deploy log https://app.netlify.com/sites/poetic-froyo-8baba7/deploys/6540a9e612bfc00008b94ce5
😎 Deploy Preview https://deploy-preview-360--poetic-froyo-8baba7.netlify.app/qdrant_client.conversions.conversion
📱 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.

@NirantK
Copy link
Contributor

NirantK commented Oct 30, 2023

The async_qdrant_xxx files are auto-generated from the base ones. The header of those files indicates so. This looks like a useful contribution. Would you mind adding this to the corresponding file in this PR itself? That might be qdrant_fastembed.py

@KaQuMiQ
Copy link
Author

KaQuMiQ commented Oct 31, 2023

Sure @NirantK ! I have missed that information 🙈 Should be fixed now

@joein
Copy link
Member

joein commented Oct 31, 2023

Hi @KaQuMiQ

Now your PR won't pass CI
There is a check which ensures that async client is up-to-date
It launches the generator and then check diffs in the async files
You can launch generator on your own (https://github.com/qdrant/qdrant-client/blob/master/tools/generate_async_client.sh)
And then push the changes, or you can leave it to us

Tests which checks whether async client is up-to-date can be found here https://github.com/qdrant/qdrant-client/blob/master/tests/async-client-consistency-check.sh

Tell me if you want to do it on your own or if you want us to finish it

Thank you!

Add fastembed cache dir argument when configuring model
@KaQuMiQ
Copy link
Author

KaQuMiQ commented Oct 31, 2023

Sure, should be fixed now @joein

@joein
Copy link
Member

joein commented Oct 31, 2023

Seems ok

The only thing that I might want to update is type hint
I'd probably prefer to have Optional[Union[str, Path]] and not just Optional[str], but for that we need to update fastembed as well (@NirantK)

@joein
Copy link
Member

joein commented Jan 7, 2024

We've put off this one for unknown reasons 🙈

Sorry for this, but thank you for your efforts!

Closing this now in favour of #416

@joein joein closed this Jan 7, 2024
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.

3 participants