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

custom destination fixes #1119

Merged
merged 8 commits into from
Mar 21, 2024
Merged

custom destination fixes #1119

merged 8 commits into from
Mar 21, 2024

Conversation

rudolfix
Copy link
Collaborator

Description

see commit list

@sh-rp you can take over this:

  • sink decorator must work without parens
  • add good docstrings to the decorators
  • write tests for:
  1. add test for _DESTINATIONS
  2. passing SPEC in the decorator
  3. do we have test for instantiated factory like pipeline.run(destination=my_gcp_sink(credentials=...)

@rudolfix rudolfix requested a review from sh-rp March 20, 2024 12:13
Copy link

netlify bot commented Mar 20, 2024

Deploy Preview for dlt-hub-docs canceled.

Name Link
🔨 Latest commit 326694c
🔍 Latest deploy log https://app.netlify.com/sites/dlt-hub-docs/deploys/65fc1035c86a1e00089c875d

sh-rp added 3 commits March 20, 2024 16:26
rename config spec
format generic destination example
remove some unneded things from the destination tests
@sh-rp sh-rp changed the title fixes sink destination base handling custom destination fixes Mar 20, 2024
@sh-rp
Copy link
Collaborator

sh-rp commented Mar 20, 2024

I think 3. is already done.

fix tests for source decorator
@@ -437,6 +451,54 @@ def my_gcp_sink(
)


def test_destination_with_spec() -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

lmk if this is enough wrt to testing the custom base

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

improved a little bit. spec is not a custom base. it is THE spec that will be used

Copy link
Collaborator Author

@rudolfix rudolfix left a comment

Choose a reason for hiding this comment

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

LGTM! you can merge @sh-rp

@@ -437,6 +451,54 @@ def my_gcp_sink(
)


def test_destination_with_spec() -> None:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

improved a little bit. spec is not a custom base. it is THE spec that will be used

@sh-rp sh-rp merged commit 1f2b4ce into devel Mar 21, 2024
42 of 47 checks passed
@AstrakhantsevaAA AstrakhantsevaAA deleted the rfix/fixes-inject-base-handling branch March 26, 2024 11:02
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