-
Notifications
You must be signed in to change notification settings - Fork 20
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
Feature/add union data #43
base: release/v0.10.0-apr-2024
Are you sure you want to change the base?
Feature/add union data #43
Conversation
…urce_relation as a unique test condition for the models
…ng and replacing the fivetran_utils entry in packages.yml file
…for oddly named netsuite2 source tables
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.
@fivetran-jamie this PR looks great! I only have one comment that I would like to discuss further, but other than that this is good to go!
.github/workflows/check_docs.yml
Outdated
@@ -0,0 +1,35 @@ | |||
name: 'check docs' |
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.
Request to remove this as we will use a different method this year to check docs.
README.md
Outdated
|
||
<details><summary><i>Expand for source configuration template</i></summary><p> | ||
|
||
> **Note**: If there are source tables you do not have (see [Step 4](https://github.com/fivetran/dbt_netsuite_source?tab=readme-ov-file#step-5-disable-models-for-non-existent-sources-netsuite2-only)), you may still include them, as long as you have set the right [variables](https://github.com/fivetran/dbt_netsuite_source?tab=readme-ov-file#step-5-disable-models-for-non-existent-sources-netsuite2-only) to `False`. Otherwise, you may remove them from your source definitions. |
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.
Same comment as the other repos. Can we remove this and try to find a more easily maintainable way to convey this information to the user.
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.
updated
# - package: fivetran/fivetran_utils | ||
# version: [">=0.4.0", "<0.5.0"] | ||
- git: https://github.com/fivetran/dbt_fivetran_utils.git | ||
revision: feature/enhance-union-data | ||
warn-unpinned: false |
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.
Reminder to swap before release
Please provide your name and company
Poonam Agate, Fivetran
Link the issue/feature request which this PR is meant to address
N/A
Detail what changes this PR introduces and how this addresses the issue/feature request linked above.
Provides the capability to union multiple Netsuite connectors together and leverage the source models in this package
How did you validate the changes introduced within this PR?
Run the models amended in BigQuery
Which warehouse did you use to develop these changes?
BigQuery
Did you update the CHANGELOG?
Did you update the dbt_project.yml files with the version upgrade (please leverage standard semantic versioning)? (In both your main project and integration_tests)
Provide an emoji that best describes your current mood
🐈
Feedback
We are so excited you decided to contribute to the Fivetran community dbt package! We continue to work to improve the packages and would greatly appreciate your feedback on our existing dbt packages or what you'd like to see next.
PR Template
Community Pull Request Template (default)
Maintainer Pull Request Template (to be used by maintainers)