Initial build #1
Conversation
There was a problem hiding this comment.
@fivetran-jamie thanks for working through this initial build. I have a handful of initial comments. Please address these as well as the remaining documentation efforts. Once those are applied I'll conduct a final review. Thanks!
| # dbt run --target "$db" | ||
| # dbt test --target "$db" | ||
| # dbt run --vars '{variable_to_test: false}' --full-refresh --target "$db" | ||
| # dbt test --target "$db" |
There was a problem hiding this comment.
Let's remove these if they're not necessary
| - name: Ask a question during our office hours | ||
| url: https://calendly.com/fivetran-solutions-team/fivetran-solutions-team-office-hours | ||
| about: Schedule time during the external office hours block with the Fivetran Analytics Engineering team for support |
There was a problem hiding this comment.
We should remove this since the Calendly link is no longer active. Also, if there was a template where this was generated from please remove it from there as well.
| description: Our team will assess this feature and let you know if we will add it to a future sprint. However, if you would like to expedite the feature, we encourage you to contribute to the package via a PR. Our team will then work with you to approve and merge your contributions as soon as possible. | ||
| options: | ||
| - label: Yes. | ||
| - label: Yes, but I will need assistance and will schedule time during your [office hours](https://calendly.com/fivetran-solutions-team/fivetran-solutions-team-office-hours) for guidance. |
There was a problem hiding this comment.
Same request as above. Please remove the hyperlink to the no longer active Calendly link as well as remove from whatever template this may have been pulled from
| description: Our team will assess this issue and let you know if we will add it to a future sprint. However, if you would like to expedite the solution, we encourage you to contribute to the package via a PR. Our team will then work with you to approve and merge your contributions as soon as possible. | ||
| options: | ||
| - label: Yes. | ||
| - label: Yes, but I will need assistance and will schedule time during our [office hours](https://calendly.com/fivetran-solutions-team/fivetran-solutions-team-office-hours) for guidance |
There was a problem hiding this comment.
Same request as the others
| jobs: | ||
| call-workflow-passing-data: | ||
| if: github.event.pull_request.merged | ||
| uses: fivetran/dbt_package_automations/.github/workflows/auto-release.yml@feature/auto-releaser |
There was a problem hiding this comment.
I believe this can now be main and not a feature branch. Please update whatever template this was pulled from to use the correct link for the auto release
| uses: fivetran/dbt_package_automations/.github/workflows/auto-release.yml@feature/auto-releaser | |
| uses: fivetran/dbt_package_automations/.github/workflows/auto-release.yml@main |
| tax_collection_model, -- always MarketplaceFacilitator in US | ||
| tax_collection_responsible_party -- always Amazon Web Services in US | ||
|
|
||
| {# columns i'm excluding -- remove later #} |
There was a problem hiding this comment.
Reminder to remove if still planning to exclude
| height, | ||
| link, | ||
| marketplace_id, | ||
| upper(variant) as variant, |
There was a problem hiding this comment.
Is there a reason we went with upper when we've taken a lower approach before? We should standardize this across this package as well as our approach across the board.
| _fivetran_synced, | ||
| asin, | ||
| marketplace_id, | ||
| item_height_unit, -- should we include both? package data seems to be more complete in general |
There was a problem hiding this comment.
Let's include both
|
|
||
| select | ||
| order_item.*, | ||
| {# Open Q: What other order/header level fields would be useful to have here? #} |
There was a problem hiding this comment.
This is a good place to start for now. We can always add more upon request
| @@ -0,0 +1,179 @@ | |||
| with item_summary as ( | |||
There was a problem hiding this comment.
Shouldn't this be dependent on the amazon_selling_partner__using_catalog_module variable?
No description provided.