🎉 New Destination: DuckDB

[sspaeti]

What

• initial version of a DuckDB destination
• I added the dbt normalization

How

• Implemented with the Python CDK very similar to destination-sqlite. The code is mostly a copy of that.

Recommended reading order

1. Read the implementation of the duckdb destination
2. check out the added dbt normalization
3. check the comment here for some specialties that didn't work without help initially.

Pre-merge Checklist

Expand the relevant checklist and delete the others.

New Connector

Community member or Airbyter

• Community member? Grant edit access to maintainers (instructions)
• Secrets in the connector's spec are annotated with airbyte_secret
• Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
• Code reviews completed
• Documentation updated
  • Connector's README.md
  • Connector's bootstrap.md. See description and examples
  • docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
  • docs/integrations/README.md
  • airbyte-integrations/builds.md
• PR name follows PR naming conventions

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

• Create a non-forked branch based on this PR and test the below items on it
• Build is successful
• If new credentials are required for use in CI, add them to GSM. Instructions.
• /test connector=connectors/<name> command is passing
• New Connector version released on Dockerhub by running the /publish command described here
• After the connector is published, connector added to connector index as described here
• Seed specs have been re-generated by building the platform and committing the changes to the seed spec files, as described here

Connector Generator

• Issue acceptance criteria met
• PR name follows PR naming conventions
• If adding a new generator, add it to the list of scaffold modules being tested
• The generator test modules (all connectors with -scaffold in their name) have been updated with the latest scaffold by running ./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates then checking in your changes
• Documentation which references the generator is updated as needed

Tests

Unit

> Task :airbyte-integrations:connectors:destination-duckdb:unitTest
Name                                Stmts   Miss  Cover
-------------------------------------------------------
destination_duckdb/__init__.py          2      0   100%
destination_duckdb/destination.py      70     48    31%
-------------------------------------------------------
TOTAL                                  72     48    33%

Deprecated Gradle features were used in this build, making it incompatible with Gradle 8.0.

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

See https://docs.gradle.org/7.6/userguide/command_line_interface.html#sec:command_line_warnings

BUILD SUCCESSFUL in 21s
33 actionable tasks: 21 executed, 12 up-to-date

Integration

> Task :airbyte-integrations:connectors:destination-duckdb:customIntegrationTests
Name                                Stmts   Miss  Cover
-------------------------------------------------------
destination_duckdb/__init__.py          2      0   100%
destination_duckdb/destination.py      70     15    79%
-------------------------------------------------------
TOTAL                                  72     15    79%

Deprecated Gradle features were used in this build, making it incompatible with Gradle 8.0.

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

See https://docs.gradle.org/7.6/userguide/command_line_interface.html#sec:command_line_warnings

BUILD SUCCESSFUL in 13s
33 actionable tasks: 15 executed, 18 up-to-date

Acceptance

Not found for Python

[sspaeti]

Converting PR back to "draft" and trying to add normalization at the same time. Any help or hints are appreciated; there are quite a lot of files touched for normalization.

I followed the most recent destination: #15592.

[sspaeti]

@edgao I finished the code review:

🎉 this is exciting! I left a number of comments. They're mostly nitpicky, but the stuff in destination.py is maybe actually important. But feel free to tell me I don't know what I'm talking about, you're the expert on this destination :)

Thank so much for your thoughtful review. I checked all cases, improved where possible, tested some things, and had to revert (all in the conversations/threads above). Now I have a working copy again with your changes. IMO it's ready to merge.

Also - when you run the integration tests locally, does it generate any sql files in normalization_test_output? (iirc it should, and you should commit those as well)

There is, but there are only two files normalization_test_output/duckdb/test_nested_streams/dbt_project.yml and normalization_test_output/duckdb/test_simple_streams/dbt_project.yml. Hope that's ok.

old and resolved

[github-actions]

Airbyte Code Coverage

There is no coverage information present for the Files changed

Total Project Coverage	24.53%	❌

[ryankfu]

There's a couple conflicting files but overall looks good. I don't see any things that I would follow up on aside from the DESTINATION_SIZE_LIMIT you mentioned before

@marcosmarxm is there anything else that I should look out for when it comes to certifying a connector? This would be the first time I've done such a review and mostly been following the building a python destination documentation

For instance, would we need to set up the integration tests internally or does the build passing give that sanity check?

[ryankfu]

LGTM