-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[ENH] CloudflareWorkersAIEmbeddingFunction #1271
[ENH] CloudflareWorkersAIEmbeddingFunction #1271
Conversation
Reviewer ChecklistPlease leverage this checklist to ensure your code review is thorough before approving Testing, Bugs, Errors, Logs, Documentation
System Compatibility
Quality
|
Hey @mileszim this is marked as draft but is it ready to be reviewed + merged? |
Probably? I haven't had time to run through the checklist. Lemme know if there is anything specific to address / scaffolding for tests or something I can bring that in. Otherwise its up to your discretion. |
@mileszim, I hope you don't mind if I do a little cleanup, testing + docs and JS EF in this PR |
- Also added a note about the max batch size. Ref: chroma-core/chroma#1271
- Removed batching, the choice and control of batching and any failures arising from is better left to the users and handled outside the EF - Added JS implementation of the EF - Added tests for both python and JS EF
There is a Cloudflare SDK for node, but given the simplicity of the API, it is overkill to include it in our depths. We can update the EF should the API change - we have the tests for it. |
…lare-workers-ai-embedding # Conflicts: # chromadb/utils/embedding_functions.py
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 now
Our underlying impl has changed and so this PR is not landable as is. That being said - we'd still like to add this functionality and that is now tracked in this issue. |
Description of changes
Summarize the changes made by this PR.
Test plan
How are these changes tested?
pytest
for python,yarn test
for jsDocumentation Changes
Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the docs repository?