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.

Sign up for GitHub

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

Jump to bottom

Add 'duckdb' support #67

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

Add 'duckdb' support #67

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e4b45c8
add duckdb for date_spline code
ericmichael Aug 7, 2023
657c065
add changelog, bump version
ericmichael Aug 7, 2023
df1c697
bump integration test version:
ericmichael Aug 7, 2023
08550a9
revert whitespace change
ericmichael Aug 7, 2023
4b84373
fix
ericmichael Aug 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
# dbt_stripe v0.10.2
## 🎉 Feature Updates
- Included support for DuckDB via ([#67](https:/fivetran/dbt_stripe/pull/67))
Copy link
Contributor

Choose a reason for hiding this comment

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

I am a bit cautious to boldly claim that this package is compatible with DuckDB since we do not have the infrastructure on our end to validate that this does in fact work for DuckDB destinations.

I would request we change the wording of the CHANGELOG entry to reflect this approach. My thoughts are something along the following:

Suggested change
- Included support for DuckDB via ([#67](https:/fivetran/dbt_stripe/pull/67))
- Included DuckDB destination in conditional logic within the `int_stripe__date_spine` model. ([#67](https:/fivetran/dbt_stripe/pull/67))

We can then claim proudly that we support DuckDB once we have it as part of our integration testing pipeline. However, for now this should be good!


## Contributors
- [ericmichael](https:/ericmichael) ([#67](https:/fivetran/dbt_stripe/pull/67))

# dbt_stripe v0.UPDATE.UPDATE

## Under the Hood:
Expand Down
2 changes: 1 addition & 1 deletion dbt_project.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
config-version: 2
name: 'stripe'

version: '0.10.1'
version: '0.10.2'
require-dbt-version: [">=1.3.0", "<2.0.0"]
models:
stripe:
Expand Down
2 changes: 1 addition & 1 deletion integration_tests/dbt_project.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
config-version: 2

name: 'stripe_integration_tests'
version: '0.10.1'
version: '0.10.2'

profile: 'integration_tests'

Expand Down
4 changes: 2 additions & 2 deletions models/intermediate/int_stripe__date_spine.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ with spine as (
{% endset %}
{% set first_date = run_query(first_date_query).columns[0][0]|string %}

{% if target.type == 'postgres' %}
{% if target.type == 'postgres' or target.type == 'duckdb' %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Small suggestion to combine postgres and duckdb in the same conditional check.

Suggested change
{% if target.type == 'postgres' or target.type == 'duckdb' %}
{% if target.type in ('postgres', 'duckdb') %}

{% set first_date_adjust = "cast('" ~ first_date[0:10] ~ "' as date)" %}

{% else %}
Expand All @@ -34,7 +34,7 @@ with spine as (
{% else %} {% set last_date = run_query(current_date_query).columns[0][0]|string %}
{% endif %}

{% if target.type == 'postgres' %}
{% if target.type == 'postgres' or target.type == 'duckdb' %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
{% if target.type == 'postgres' or target.type == 'duckdb' %}
{% if target.type in ('postgres', 'duckdb') %}

{% set last_date_adjust = "cast('" ~ last_date[0:10] ~ "' as date)" %}

{% else %}
Expand Down