Bugfix/redshift constant expressions #91
Conversation
There was a problem hiding this comment.
@fivetran-reneeli thanks for working through these changes. I have a few change requests and comments below. Let me know if you want to sync on any of these. Particularly if you have any comments around my questions regarding the metafield approach.
Co-authored-by: Joe Markiewicz <[email protected]>
|
Thanks @fivetran-joemarkiewicz ! Let me know what you think, especially regarding the new metafield config and the readme updates. |
There was a problem hiding this comment.
@fivetran-reneeli this is looking great, just a few more changes before approval.
README.md
Outdated
| The package takes into consideration that not every Shopify connector may have the `fulfillment_event`, `metadata`, or `abandoned_checkout` tables (including `abandoned_checkout`, `abandoned_checkout_discount_code`, and `abandoned_checkout_shipping_line`) and allows you to enable or disable the corresponding functionality. | ||
|
|
||
| The `fulfillment_event` table is **disabled by default** but does hold valuable information that is leveraged in the `shopify__daily_shop` model in the transformation package. If you would like to enable the modeling of fulfillment events downstream, add the following variable to your `dbt_project.yml` file: | ||
|
|
||
| Add the following variable to your `dbt_project.yml` file to enable the modeling of fulfillment events: | ||
| ```yml | ||
| # dbt_project.yml | ||
|
|
||
| vars: | ||
| shopify_using_fulfillment_event: true # false by default | ||
| ``` | ||
|
|
||
| The `metadata` table is **enabled by default**. To disable this tables and prevent metadata fields from persisting downstream, add the following variable to your `dbt_project.yml` file: | ||
|
|
||
| ```yml | ||
| # dbt_project.yml | ||
|
|
||
| ... | ||
| vars: | ||
| shopify_using_metafield: false #true by default | ||
| ``` | ||
|
|
||
| The `abandoned_checkout` tables (including `abandoned_checkout` in addition to the `abandoned_checkout_discount_code` and `abandoned_checkout_shipping_line` child tables) are **enabled by default**. To disable the `abandoned_checkout` tables, add the following variable to your `dbt_project.yml` file: | ||
|
|
||
| ```yml | ||
| # dbt_project.yml | ||
|
|
||
| ... | ||
| vars: | ||
| shopify_using_abandoned_checkout: false #true by default | ||
| ``` |
There was a problem hiding this comment.
This is unnecessarily long and broken out. Let's consolidate them into one enable/disable section like we do in our other packages. For example:
### Step 4: Disable models for non-existent sources
The package takes into consideration that not every Shopify connector may have the `fulfillment_event`, `metadata`, or `abandoned_checkout` tables (including `abandoned_checkout`, `abandoned_checkout_discount_code`, and `abandoned_checkout_shipping_line`) and allows you to enable or disable the corresponding functionality. If you would like to enable/disable the modeling of above mentioned source and their downstream references, add the following variable to your `dbt_project.yml` file:
vars:
shopify_using_fulfillment_event: true # false by default. Enables the fulfillment_event source
shopify_using_metafield: false #true by default. Disables the metafield source
shopify_using_abandoned_checkout: false #true by default. Disables the abandoned_checkout, abandoned_checkout_discount_code, and abandoned_checkout_shipping_line sources
There was a problem hiding this comment.
This way it's all in one place and customers don't need to scan over multiple blocks.
There was a problem hiding this comment.
Ahh good point. I shortened it into 1 section
CHANGELOG.md
Outdated
| # dbt_shopify_source v0.13.0 | ||
|
|
||
| [PR #91](https://github.com/fivetran/dbt_shopify_source/pull/91) includes the following changes: | ||
| ## Under the Hood |
There was a problem hiding this comment.
| ## Under the Hood | |
| ## Breaking Changes |
I would call this or at least part of it "breaking changes" since we bumped the minor version and have no other changes.
Co-authored-by: fivetran-catfritz <[email protected]>
To summarize the internal discussion with the team:
metafieldorabandoned_checkout_discount_codetables. Therefore we added enable/disable configs for these so that they may be disabled if not used by the customer.PR Overview
This PR will address the following Issue/Feature: #90
This PR will result in the following new package version: v0.13.0
Potential changes to schemas
Please provide the finalized CHANGELOG entry which details the relevant changes included in this PR:
Under the Hood
metadatastaging model (stg_shopify__metafield).abandoned_checkoutstaging models (includingstg_shopify__abandoned_checkout,stg_shopify__abandoned_checkout_discount_code, andstg_shopify__abandoned_checkout_shipping_line).PR Checklist
Basic Validation
Please acknowledge that you have successfully performed the following commands locally:
Before marking this PR as "ready for review" the following have been applied:
Detailed Validation
Please share any and all of your validation steps:
Recreate the constant expressions error in integration_test folder by running on an empty schema ( ex:
shopify_schema: empty). If the upstream table is empty, it should be disabled.If you had to summarize this PR in an emoji, which would it be?
💃