-
Notifications
You must be signed in to change notification settings - Fork 0
V0.14.0 fixes #47
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
V0.14.0 fixes #47
Conversation
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.
⚠️ API Contract Change Detected
This PR modifies the public API contract of the CyborgDB JS SDK.
Please provide an explanation for this change:
- Why is this change necessary?
- Is this a breaking change or backward compatible?
- Have you updated the documentation?
This review must be dismissed or addressed before the PR can be merged.
src/__tests__/api_contract.test.ts
Outdated
| // Contents should be decoded to string by SDK | ||
| expect(typeof result.contents).toBe('string'); | ||
| // Contents for the ids were upserted as buffer object | ||
| expect(typeof result.contents).toBe('object'); |
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.
The Python SDK expects contents to be returned as a string, even when uploaded as bytes but for JS now we are expecting it to be as raw buffer object? I think we should have consistent behavior across both sdks, having text content is more user friendly
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.
revert it back for now
the changes were to reduce the amount of failed test when running them and to fix one bug for upsert. |
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.
bug fix for upsert if we upsert without vector but also include a context
added better error handling for upsert and update the test to be more accurate to what we're upserting