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

fix/chroma destination experimental fix #297

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

mateuszkuprowski
Copy link
Contributor

@mateuszkuprowski mateuszkuprowski commented Dec 11, 2024

This is a very small unbreaking fix for chroma destination. I have a suspiction that under some circumstances we get both non-empty Path and host and port. In that case the most likely cause is Path arriving as an empty string which causes PersistentClinet instead of HttpClient to fire. I have reversed checking order of clients for experimental purpose. This shouldn't break any current logic.

hubert-rutkowski85 and others added 16 commits December 2, 2024 17:49
* add release branch to PR triggers

* omit vectar dest e2e test
* create deterministic id for upload use

* fix id in sql connector
Few fsspec connectors: SFTP, Azure, Box and GCS havedate_modified and date_created fields of FileDataSourceMetadata class were of type float | None instead of str | None, modified code creating the metadata to cast float timestamps to strings.

---------

Co-authored-by: Filip Knefel <[email protected]>
* Snowflake query data binding fix

* Enable SingleSource source connector entry

* Fix Snowflake nan issue

* Make Singlestore connector more robust against SQL injection

* Clean sql upload and add query debug log

* Make SQLite connector more robust against sql injection

* SQL injection fixes; Changelog and version update

* Optimize memory usage of Snowflake uploader

* Changelog update: Optimize memory usage of Snowflake uploader
* Fix Snowflake downloader

* Changelog and version update: Fix Snowflake downloader

* Replace Snowflake source connector inheritance with SQL classes

* Comment on snowflake dependency name

* Get rid of snowflake postgres inheritance. Replaced with SQL.

* Fix lint

* Version update: Fix Snowflake downloader
Refined box connector to actually use config JSON directly
---------

Co-authored-by: Mateusz Kuprowski <[email protected]>
Co-authored-by: Michal Martyniak <[email protected]>
When run on windows Path(<path-object>) converts slashes to backward slashes which are not correctly interpreted when passed to (non-local) fsspec filesystem.
Instead of using str(<path-object>) use <path-object>.to_posix() to mitigate this effect in fsspec code.

---------

Co-authored-by: Filip Knefel <[email protected]>
Original error logging was never called because by default parallel_bulk re-raises exceptions and raises errors for non 2XX responses and these were not caught.

We change the logic to catch, log and re-raise errors on our side. Error log is sanitized to remove the uploaded object contents from it.


---------

Co-authored-by: Filip Knefel <[email protected]>
Update Weaviate connector example

---------

Co-authored-by: Filip Knefel <[email protected]>
Copy link
Contributor

@potter-potter potter-potter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like a reasonable fix

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.

6 participants