[CLN] Separate validation and transformation logic #2899
+933
−675
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
|
Please tag your PR title with one of: [ENH | BUG | DOC | TST | BLD | PERF | TYP | CLN | CHORE]. See https://docs.trychroma.com/contributing#contributing-code-and-ideas |
Reviewer ChecklistPlease leverage this checklist to ensure your code review is thorough before approving Testing, Bugs, Errors, Logs, Documentation
System Compatibility
Quality
|
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
atroyn
changed the title
atroyn
force-pushed
the
anton/separate_validation_logic
branch
12 times, most recently
from
4f829fc
to
9d25975
Compare
atroyn
force-pushed
the
anton/separate_validation_logic
branch
4 times, most recently
from
b948c07
to
440e3b2
Compare
atroyn
commented
Oct 19, 2024
| coll.add(ids=ooo_ids, embeddings=embeddings) | ||
| get_ids = coll.get(ids=ooo_ids)["ids"] | ||
| assert get_ids == ooo_ids | ||
|
|
||
|
|
||
| @pytest.mark.xfail(reason="Partial records aren't in our API contract...") |
There was a problem hiding this comment.
I don't think we actually intend to support this? This is very old code and I'm not sure it's used anywhere.
atroyn
force-pushed
the
anton/separate_validation_logic
branch
from
440e3b2
to
b91c9e5
Compare
atroyn
force-pushed
the
anton/separate_validation_logic
branch
from
b91c9e5
to
5d181a9
Compare
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
We perform validation, and transformations, on requests before we send them from the client to the server. Previously, these operations were not well modularized, leading to difficult to debug code, lots of exceptional paths, and overall not-niceness.
This PR separates validation, and makes it idemponent, from transformations. For each type of request against our API.
Along the way, this PR cleans up the possibility of sending empty lists or dicts as filters or lists of IDs, so that the tests are now consistent with our validators.
In a future PR, we will also repeat this same validation logic server-side for use with clients other than our first party.
Test plan
CI Passes
New validation to ensure empty filters trigger validators.
Documentation Changes
TODO