update readme to include how to use metadata variable dictionary arguments #57
Conversation
fivetran-reneeli
changed the base branch from
main
to
MagicBot/dbt-utils-cross-db-migration
There was a problem hiding this comment.
@fivetran-reneeli small wording request change to remove the verbiage around Fivetran Utils since that will already be handled for them.
README.md
Outdated
|
|
||
| If you happen to be using a reserved word as a field in your metadata, similarly incompatible name, or just wish to rename your field, we have updated our variables to be able to accept a dictionary in addition to strings, via using aliases. Below is an example using `stripe__plan_metadata` of how you would add the respective variables to your root `dbt_project.yml` file. Please note you must have at least fivetran_utils v0.4.0 or higher. |
There was a problem hiding this comment.
Once this is rolled out the customer will not be able to control the fivetran_utils version. We can probably remove the last line in this sentence.
README.md
Outdated
| ```yml | ||
| packages: | ||
| - package: fivetran/fivetran_utils | ||
| version: ">0.4.0" | ||
| ``` |
There was a problem hiding this comment.
We should remove this as fivetran_utils 0.4.0 will be installed automatically
…bt_stripe_source into feature/json_metadata
There was a problem hiding this comment.
@fivetran-reneeli I just have a small comment about removing duplicating in the README and making it consistent and clear for the customers. Let me know if you have questions.
README.md
Outdated
| **Note** | ||
|
|
||
| Alternatively, if you happen to be using a reserved word as a field in your metadata, similarly incompatible name, or just wish to rename your field, we have updated our variables to be able to accept a dictionary in addition to strings. Below is an example using `stripe__plan_metadata` of how you would add the respective fields to your root `dbt_project.yml` file. | ||
|
|
||
| ```yml | ||
| vars: | ||
| stripe__plan_metadata: | ||
| - metadata_field_1 | ||
| - metadata_field_2 | ||
| - name: incompatible.field | ||
| alias: rename_incompatible_field | ||
| - name: field_is_reserved_word | ||
| alias: field_is_reserved_word_xyz | ||
| ``` |
There was a problem hiding this comment.
I think this should replace the documentation for the metadata variables that you see above. I feel it is a bit duplicative. We could still include the note about the reserved word, but this way it is condensed into one section.
Similar to how we document the variables here. We should commit to instructing the customer to either write the variable. While we can support the ability to write the old way, we should instruct people to only write the variable in the new way. With the name or the name and the alias
There was a problem hiding this comment.
Just updated to condense with the prior info we had for metadata variables.
|
@fivetran-joemarkiewicz ready for re review |
There was a problem hiding this comment.
@fivetran-reneeli the condensed update looks great. I did have one last suggestion I would like for you to apply to the README before merging. Let me know if you have any questions around my final suggestion.
I like your addition of the "variable accepts dictionary and strings", but for purposes of people scanning and looking at the variables, I think it is best to be consistent with how they should declare the variables.
Co-authored-by: Joe Markiewicz <[email protected]>
Makes sense to keep it standard to not raise any confusion. Changes applied! |
Pull Request
Are you a current Fivetran customer?
internal
What change(s) does this PR introduce?
update readme to include how to use metadata variable dictionary arguments
Did you update the CHANGELOG?
Does this PR introduce a breaking change?
updating readme only
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)
Is this PR in response to a previously created Bug or Feature Request
How did you test the PR changes?
Select which warehouse(s) were used to test the PR
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.