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

feat(destinations): implement destination_info property #1534

Open
wants to merge 10 commits into
base: devel
Choose a base branch
from

Conversation

IlyaFaer
Copy link
Contributor

@IlyaFaer IlyaFaer commented Jul 2, 2024

Towards #751

Implements item 2

Copy link

netlify bot commented Jul 2, 2024

Deploy Preview for dlt-hub-docs ready!

Name Link
🔨 Latest commit ce291ea
🔍 Latest deploy log https://app.netlify.com/sites/dlt-hub-docs/deploys/66c305b39c468f0008dad58f
😎 Deploy Preview https://deploy-preview-1534--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.

@IlyaFaer
Copy link
Contributor Author

IlyaFaer commented Jul 2, 2024

@rudolfix, made a draft for the item 2 (you said we have to talk about it first) to see if it's close to what was requested.

dlt/common/destination/reference.py Outdated Show resolved Hide resolved
dlt/destinations/impl/destination/configuration.py Outdated Show resolved Hide resolved
tests/destinations/test_custom_destination.py Outdated Show resolved Hide resolved
@IlyaFaer IlyaFaer marked this pull request as ready for review July 11, 2024 07:23
@IlyaFaer
Copy link
Contributor Author

IlyaFaer commented Jul 11, 2024

@rudolfix, oh, so in dlt repo there are no secrets for destinations? Hm-m. This test is supposed to be moved to verified-sources then?! But this requires access to dlt.tests.utils.py, which is not included into the package build. Yeah, I see... But I guess IMPLEMENTED_DESTINATIONS can be replaces with ALL_DESTINATIONS in verified-sources.

Locally, all the tests are passing, except two, because I don't have credentials for them:

image

So, I guess, this one can be merged, and I'll move the tests into verified-sources

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.

2 participants