[ENH] multiple api keys support #2987
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Deleted unnecessary "type: ignore" comments that were causing unused-ignore errors. - Ensured that the code is properly typed without the need for ignoring type checks.
- Refactored OpenAIEmbeddingFunction to use an instance variable for the API key. - Removed direct assignment to global openai.api_key to prevent unintended side effects. - Each instance can now maintain its own API key, allowing for more flexible usage. - Added fallback to global openai.api_key if no key is provided during instantiation.
Reviewer ChecklistPlease leverage this checklist to ensure your code review is thorough before approving Testing, Bugs, Errors, Logs, Documentation
System Compatibility
Quality
|
## Description of changes Improvements & Bug fixes - Add `@rate_limit` decorator in front of all server-side operations for the HTTP API ## Test plan *How are these changes tested?* - [ ] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust ## Documentation Changes *Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the [docs repository](https://github.com/chroma-core/docs)?*
## Description of changes - Improvements & Bug fixes - We are providing a "fast" storage path on our cloud instances at `/local` so this needs to be reflected in our Helm charts as the `hostPath` for our cache directory. ## Test plan *How are these changes tested?* - [x] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust `tilt up` builds and updates the charts in the expected way. ## Documentation Changes *Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the [docs repository](https://github.com/chroma-core/docs)?*
…core#2989) ## Description of changes Improvements & Bug fixes - After an audit of everywhere that uses an instance of `ServerAPI`, there were a few places where `tenant`/`database` were not being threaded all the way through. This PR fixes that. ## Test plan *How are these changes tested?* - [ ] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust ## Documentation Changes *Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the [docs repository](https://github.com/chroma-core/docs)?*
## Description of changes Improvements & Bug fixes - Adds a new ChromaError, QuotaError. Meant to be raised when a client exceeds given quotas. ## Test plan *How are these changes tested?* - [ ] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust ## Documentation Changes *Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the [docs repository](https://github.com/chroma-core/docs)?*
## Description of changes * This log line logs every record id in the candidate set, which is just a pain to read. Instead, log the size of the set. ## Test plan *How are these changes tested?* - [ ] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust ## Documentation Changes *Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the [docs repository](https://github.com/chroma-core/docs)?*
## Description of changes * We only had tracing for the `worker` crate. Enable it for all our crates at trace level and info level for non-chroma crates. ## Test plan *How are these changes tested?* - [ ] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust ## Documentation Changes *Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the [docs repository](https://github.com/chroma-core/docs)?*
…2996) ## Description of changes *Summarize the changes made by this PR.* - Improvements & Bug fixes - ... - New functionality - ... ## Test plan *How are these changes tested?* - [ ] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust ## Documentation Changes *Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the [docs repository](https://github.com/chroma-core/docs)?*
The RUST_LOG environment variable will allow the environment to override the specifics of which crates log at what level. The format of RUST_LOG is: default_level,crate_name1=some_level,crate_name2=some_other_level
## Description of changes *Summarize the changes made by this PR.* - RPC errors in sysdb were set in the response protobufs of sysdb in the "status" field. Upstream consumers in a few places don't check this status field at all and thus ignore errors from sysdb. In one case, this led to the log service deleting collections because it incorrectly assumed a collection to be not present whereas in reality the rpc had failed. This PR deprecates this field from the protobuf and instead transforms the error to a grpc error that grpc sends to the consumers separately - Also, added better logging messages in the sysdb. It's a piece of trash rn - Please note that I have deliberately not made this backwards compatible. The only issues that can arise would be during rolling upgrade deployment for the first version that has this change and given our scale, I am comfortable with dealing with it. Since this change is very fundamental to how we propagate errors in grpc instead of maintaining this field and its backward compatibility for the foreseeable future, the sooner we do it the right way the better ## Test plan - [x] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust ## Documentation Changes None
## Description of changes Improvements & Bug fixes - Make `QuotaEnforcer` naive, completely left to implementation specifics ## Test plan *How are these changes tested?* - [ ] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust ## Documentation Changes *Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the [docs repository](https://github.com/chroma-core/docs)?*
…hroma-core#3006) ## Description of changes * Fix that by removing chroma from list of traced crates. ## Test plan *How are these changes tested?* - [X] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust ## Documentation Changes *Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the [docs repository](https://github.com/chroma-core/docs)?*
## Description of changes *Summarize the changes made by this PR.* - Improvements & Bug fixes - Flush all blocks in parallel using `FuturesUnordered` - MetadataSegment flushing can take very long for large collections, this should speed it up - New functionality - None ## Test plan *How are these changes tested?* Existing tests, this is a nonfunctional change - [x] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust ## Documentation Changes None
I need a feature that's only present in the latest libraries. Upgrade to get it. ## Description of changes * Should be a NOP until the next patch. ## Test plan *How are these changes tested?* - [✅] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust ## Documentation Changes *Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the [docs repository](https://github.com/chroma-core/docs)?*
Release 0.5.16
## Description of changes * Filter OTEL exporter to be traces over 1ms or non-foyer only * Filter bunyan stdout exporter to be only the clear ## Test plan *How are these changes tested?* Localhost with jaeger
## Description of changes *Summarize the changes made by this PR.* - Improvements & Bug fixes - The NAC doesn't limit write path, only read. Which was my misunderstanding of whats finished. This is a temporary workaround to limit concurrency. - New functionality - None ## Test plan *How are these changes tested?* - [x] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust ## Documentation Changes None
## Description of changes Basic chat-with-your-docs-example ## Test plan Proofread and follow manually ## Documentation Changes Included
## Description of changes Add production banner to deployments index page ## Test plan N/A ## Documentation Changes N/A
## Description of changes *Summarize the changes made by this PR.* - Improvements & Bug fixes - Added more reference to the binary search algorithm we implemented based on `std::slice::binary_search` - New functionality - ... ## Test plan *How are these changes tested?* - [x] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust ## Documentation Changes *Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the [docs repository](https://github.com/chroma-core/docs)?* --------- Co-authored-by: Sicheng Pan <[email protected]>
## Description of changes This adds an ordered blockfile writer mode, allowing for far more efficient writes when your data is already in sorted order. A few interface changes: - There are now two delta implementations. `DeltaCommon` is a new trait implemented by all delta impls. - The block manager is delta type agnostic, it takes a generic parameter when it needs a specific delta type. There is no dynamic dispatch. - The storage builder is passed directly to the `ArrowWritableValue` implementations. We will be able to get rid of the delta layer entirely after Hammad's changes (requires making `BlockfileWriter` generic over K & V). The deltas are currently split at commit time for simplicity, but this can be changed in the future. A follow-up PR will add benchmarks for both writer modes. Remaining to do before merge: - [ ] ~~resolve remaining `todo` comments (see GitHub comments for discussion)~~ - [ ] optional: split up proptest into two tests to run ordered and unordered tests in parallel ## Test plan *How are these changes tested?* - [x] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust ## Documentation Changes *Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the [docs repository](https://github.com/chroma-core/docs)?* n/a
## Description of changes Improvements & Bug fixes - Updates the signatures on `QuotaEnforcer` abstract class and `SimpleQuotaEnforcer` impl slightly to explicitly name the parameters on `enforce()`. Adds the `set_context()` method. - Sets up the relevant context passing and quota enforcement within the API. ## Test plan *How are these changes tested?* - [ ] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust ## Documentation Changes *Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the [docs repository](https://github.com/chroma-core/docs)?*
## Description of changes *Summarize the changes made by this PR.* - Improvements & Bug fixes - Use typed uuid for segment id - New functionality - N/A ## Test plan *How are these changes tested?* - [x] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust ## Documentation Changes *Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the [docs repository](https://github.com/chroma-core/docs)?* N/A --------- Co-authored-by: Sicheng Pan <[email protected]>
## Description of changes Improvements & Bug fixes - Add Kubernetes pod name to all traces ## Test plan *How are these changes tested?* - [ ] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust ## Documentation Changes *Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the [docs repository](https://github.com/chroma-core/docs)?*
rescrv
changed the title
|
I've updated the title of the PR to pass the checks we have. You'll have to rebase and pick up the latest changes to pass the remaining pieces of CI. Happy Hacking, |
1753 / 10k spans are the insert in one space that I looked. This should silence them without making us fly blind.
…hroma-core#3096) ## Description of changes This PR enhances memory efficiency and error handling in worker endpoints for querying vectors and metadata. *Summarize the changes made by this PR.* - Improvements & Bug fixes - Memory Optimization: Preallocates vectors when sizes are known, reducing memory overhead and enhancing efficiency. - Error Handling Refactor: Consolidates common validation logic and removes redundant checks, simplifying error handling. - Enhanced Testing: Adds unit tests to ensure request validation and improve reliability. - New functionality - ... ## Test plan *How are these changes tested?* Added unit tests for request validation. No new functionality was introduced, so existing tests cover other impacted areas - [ ] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust `cargo test` ## Documentation Changes *Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the [docs repository](https://github.com/chroma-core/docs)?*
…re#3043) ## Description of changes Increases throughput by 11x for fresh blockfiles (and, I suspect, on forked blockfiles where the new set of data is mostly disjoint from the forked blocks). Does not result in a significant performance difference when mostly rewriting blocks on a forked blockfile. Also increases throughput when writing to forked blockfiles by ~10% by avoiding clones and excess comparisons. <img width="823" alt="Screenshot 2024-11-05 at 4 20 08 PM" src="https://github.com/user-attachments/assets/210855f8-94ec-4398-9382-d136274739ed"> ## Test plan *How are these changes tested?* - [x] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust ## Documentation Changes *Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the [docs repository](https://github.com/chroma-core/docs)?* n/a
## Description of changes *Summarize the changes made by this PR.* - Improvements & Bug fixes - chroma-core#2998 led to a flaky test. This PR switches to the old code path for vector query - New functionality - ... ## Test plan *How are these changes tested?* - [ ] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust ## Documentation Changes *Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the [docs repository](https://github.com/chroma-core/docs)?* --------- Co-authored-by: Sicheng Pan <[email protected]>
…#3038)" (chroma-core#3107) This reverts commit fc54f70. Which does not quite work the way I'd like because I can't disambiguate between a real disconnect and a fake one. ## Description of changes *Summarize the changes made by this PR.* - Improvements & Bug fixes - None - New functionality - None ## Test plan *How are these changes tested?* - [x] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust ## Documentation Changes None
…ring (chroma-core#3110) ## Description of changes *Summarize the changes made by this PR.* - Improvements & Bug fixes - The block manager now writes to the cache when compacting. This should reduce I/O time on subsequent compactions. - Removes cache purging for blocks at the end of compaction, this is a hack and we should tune caches to not OOM instead. - New functionality - None ## Test plan *How are these changes tested?* - [x] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust ## Documentation Changes None
## Description of changes Improvements & Bug fixes - Change the `QuotaError` status code from 429 to 400. ## Test plan *How are these changes tested?* - [ ] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust ## Documentation Changes *Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the [docs repository](https://github.com/chroma-core/docs)?*
…oma-core#3115) ## Description of changes *Summarize the changes made by this PR.* - Improvements & Bug fixes - The cache code uses weighted capacity and specifies the capacity in MiB units. But we currently assume blocks are 1 MB. - The underlying foyer cache is capacity and weight based. Fixing to MiB makes cache configuration indirect from the mechanics of the cache and results in it being cumbersome and potentially outright impossible to use. For example if I want to limit to 7000 items. I have to specify a very small value as my capacity. This value may not be an integer but the configuration expects an integer capacity. Or alternatively, I can say that each item has some byte weight and choose a limit based on that. Rather than force these workarounds, it makes more sense for callers to think in whatever units they need and make the cache specification agnostic to that. As is, if a cache does not want to think in MiB and instead wants to think in items, it cannot without complication. - New functionality - None ## Test plan *How are these changes tested?* - [x] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust ## Documentation Changes None
- Deleted unnecessary "type: ignore" comments that were causing unused-ignore errors. - Ensured that the code is properly typed without the need for ignoring type checks.
- Refactored OpenAIEmbeddingFunction to use an instance variable for the API key. - Removed direct assignment to global openai.api_key to prevent unintended side effects. - Each instance can now maintain its own API key, allowing for more flexible usage. - Added fallback to global openai.api_key if no key is provided during instantiation.
This reverts commit 1c378fa.
…beycr7/chroma into feature/multiple-api-keys-support
|
It seems that I made some mistakes when rebasing onto the main branch. As a result, my commit history became cluttered, so I'm going to close this pull request and create a new one. Apologies for the inconvenience, and thank you for your patience. I'll let you know when I create a new pull request! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of changes
Summarize the changes made by this PR.
Allow multiple openAI API keys [Bug]: Global Overwrite of OpenAI API Key During Text Embedding Execution #2979
Removed direct assignment to global openai.api_key to prevent unintended side effects.
Test plan
How are these changes tested?
pytestfor python,yarn testfor js,cargo testfor rustIt seems that
cargo testis failing due to a build issue. #2294Both
pytestandyarn testsucceed.Documentation Changes
Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the docs repository?