Skip to content

Conversation

@rudolfix
Copy link
Collaborator

Description

This is breaking change :/ that got slipped. Some modules were fixed, some not (weaviate example does not have tests)

@rudolfix rudolfix requested review from burnash and sh-rp December 23, 2023 18:24
@netlify
Copy link

netlify bot commented Dec 23, 2023

Deploy Preview for dlt-hub-docs ready!

Name Link
🔨 Latest commit 9ed2fb2
🔍 Latest deploy log https://app.netlify.com/sites/dlt-hub-docs/deploys/6589b3d254bd7a000858df2a
😎 Deploy Preview https://deploy-preview-854--dlt-hub-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@burnash
Copy link
Collaborator

burnash commented Dec 23, 2023

@rudolfix what is the rationale for moving adapters to dlt.destination.adapters? Could you elaborate on the background?

@rudolfix
Copy link
Collaborator Author

@burnash we could not keep backward compatibility because the destinations are not modules but classes now (so we can parametrize them easily).
this makes it impossible to import ie from dlt.destinations.weaviate. callable modules are pure hacks and I'm still not 100% sure it is possible to import from them.

I think putting adapters in a separate namespace is a good compromise now. I expect all destinations to have adapters (ie. #855 ). the alternative is to import form impl (which someone did to Qdrant)

This slipped through. In the future we must be more aware of non compatible changes...

@rudolfix rudolfix merged commit 9a8b239 into devel Dec 29, 2023
@rudolfix rudolfix deleted the rfix/adds-adapters-namespace branch December 29, 2023 11:03
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.

3 participants