-
Notifications
You must be signed in to change notification settings - Fork 2
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
Release/0.4.0 #10
base: main
Are you sure you want to change the base?
Release/0.4.0 #10
Changes from all commits
60cbeb4
acae74d
1c16418
1ba61da
0a16fdf
e8e8cfb
8b0a3d7
e56743d
d131790
cd72d7a
12457d4
5b008da
53570f1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
This file was deleted.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For this change there is no need for a set of validation tests since this is not directly making any changes to the underlying model logic. However, would you be able to add the following model config so we can use it for review testing and future validation testing once we add it to this package. models:
+schema: "twilio_{{ var('directed_schema','dev') }}" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that's a good point, thanks for dropping that config! i added |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
name: 'twilio_integration_tests' | ||
version: '0.3.0' | ||
version: '0.4.0' | ||
config-version: 2 | ||
profile: 'integration_tests' | ||
|
||
|
@@ -14,11 +14,13 @@ vars: | |
twilio_usage_record_identifier: "twilio_usage_record_data" | ||
twilio_call_identifier: "twilio_call_data" | ||
twilio_message_identifier: "twilio_message_data" | ||
|
||
dispatch: | ||
- macro_namespace: dbt_utils | ||
search_order: ['spark_utils', 'dbt_utils'] | ||
|
||
models: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. More of a question: There weren't any validation tests added, so was this just for future use? Did we think it wouldn't have been useful to add a row comparison test given the changes weren't significant enough to the code? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah! Yup just for future use. No validation tests for this PR since this isn't really changing any model. |
||
+schema: "twilio_{{ var('directed_schema','dev') }}" | ||
|
||
seeds: | ||
twilio_integration_tests: | ||
+column_types: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,20 +3,23 @@ | |
with messages as ( | ||
|
||
select * | ||
from {{ ref('int_twilio__messages')}} | ||
from {{ ref('int_twilio__messages') }} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for making these formatting changes to ensure we have consistent formatting! |
||
), | ||
|
||
account_history as ( | ||
|
||
select * | ||
from {{ var('account_history')}} | ||
), | ||
from {{ var('account_history') }} | ||
) | ||
|
||
{% if var('using_twilio_usage_record', True) %} | ||
, | ||
usage_record as ( | ||
|
||
select * | ||
from {{ var('usage_record')}} | ||
from {{ var('usage_record') }} | ||
) | ||
{% endif %} | ||
|
||
select | ||
messages.account_id, | ||
|
@@ -44,13 +47,18 @@ select | |
{% endfor %} | ||
|
||
count(messages.message_id) as total_messages, | ||
round( cast(sum(messages.price) as {{ dbt.type_numeric ()}}), 2) * -1 as total_messages_spend, | ||
round( cast(sum(usage_record.price) as {{ dbt.type_numeric ()}}), 2) as total_account_spend | ||
round( cast(sum(messages.price) as {{ dbt.type_numeric() }}), 2) * -1 as total_messages_spend | ||
|
||
{% if var('using_twilio_usage_record', True) %} | ||
, round( cast(sum(usage_record.price) as {{ dbt.type_numeric() }}), 2) as total_account_spend | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you update the documentation for this field here to add a disclaimer that this will only be present if the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added |
||
{% endif %} | ||
|
||
from messages | ||
{% if var('using_twilio_usage_record', True) %} | ||
left join usage_record | ||
on messages.account_id = usage_record.account_id | ||
and messages.date_day = usage_record.start_date | ||
{% endif %} | ||
left join account_history | ||
on messages.account_id = account_history.account_id | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,18 +4,6 @@ with messages as ( | |
from {{ ref('int_twilio__messages') }} | ||
), | ||
|
||
incoming_phone_number as ( | ||
|
||
select * | ||
from {{ var('incoming_phone_number')}} | ||
), | ||
Comment on lines
-7
to
-11
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wow I didn't realize incoming phone number was included here as well even with it not being referenced. Thanks for removing! |
||
|
||
addresses as ( | ||
|
||
select * | ||
from {{ var('address')}} | ||
), | ||
|
||
{% if var('using_twilio_messaging_service', True) %} | ||
messaging_service as ( | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,7 @@ | ||
packages: | ||
- package: fivetran/twilio_source | ||
version: [">=0.3.0", "<0.4.0"] | ||
# - package: fivetran/twilio_source | ||
# version: [">=0.3.0", "<0.4.0"] | ||
|
||
- git: https://github.com/fivetran/dbt_twilio_source.git | ||
revision: usage_record_enable_config | ||
warn-unpinned: false | ||
Comment on lines
+2
to
+7
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reminder to update before merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
our standard way would be
twilio__using_usage_record
, but then I should also updateusing_twilio_call
andusing_twilio_messaging_service
, which would require users who already have those variables configured to change it, which would be more of a hassle customer-experience wise.