add find and replace to json extract

[fivetran-reneeli]

What change does this PR introduce?

Add a find+replace to our pivot_json_extract macro, similar to (' ', '_') but with ('.', '_').

If this PR introduces a new macro, how did you test the new macro?

n/a

If this PR introduces a modification to an existing macro, which packages is the macro currently present in and what steps were taken to test compatibility across packages?

Stripe
Issue fivetran/dbt_stripe#45
Testing with dbt compile -t snowflake -m stg_stripe__plan and dbt run -t snowflake -m stg_stripe__plan

Did you update the README to reflect the macro addition/modifications?

• Yes
• No (provide further explanation)

[fivetran-joemarkiewicz]

@fivetran-reneeli your approach here makes sense and definitely addresses the feature.id type metadata fields that the customer was experiencing. However, as you pointed out, it won't solve the reserved word error or other errors we may not be aware of at the moment.

I was able to do some digging and think I found a unique workaround that can address both the . issue and the reserved word error. See below for the modified macro code I added to make this work:

{% macro pivot_json_extract(string, list_of_properties) %}

{%- for property in list_of_properties -%}
{%- if property is not string -%}
replace( {{ fivetran_utils.json_extract(string, property.name) }}, '"', '') as {{ property.alias if property.alias else property.name | replace(' ', '_') | replace('.', '_') | lower }}

{%- else -%}
replace( {{ fivetran_utils.json_extract(string, property) }}, '"', '') as {{ property | replace(' ', '_') | lower }}

{%- endif -%}
{%- if not loop.last -%},{%- endif %}
{% endfor -%}

{% endmacro %}

With this update to the macro, we allow the variable to accept a dictionary in addition to the normal string behavior. This means, if a customer wants to alias the fields or is experiencing a reserved word or incompatible name, then they can change it to their liking!

I was able to get this to work using the following variable config:

  stripe__plan_metadata:
    - limit
    - version
    - name: feature.id
      alias: feature_id
    - name: plan.handle
      alias: plan_handle
    - name: set
      alias: sets

I feel we should update the macro for the next iteration of fivetran_utils to follow this behavior. This could then address the customers issue. We would just want to update the dbt_stripe and dbt_stripe_source README's to show how a customer may leverage these new dictionary arguments if needed.

[fivetran-reneeli]

Hi @fivetran-joemarkiewicz , I updated the readmes in the stripe packages to reflect these new instructions. Let me know if they make sense!
stripe
stripe source

[fivetran-joemarkiewicz]

Thanks @fivetran-reneeli I noticed this PR didn't apply the updates to the macro. Would you be able to make those updates as well?

[fivetran-joemarkiewicz]

@fivetran-reneeli I had one comment that I included in the notes below. Can you test this on your end and make sure it works as expected.

Additionally, be sure to add a CHANGELOG entry. Let me know once the updates are applied and I can re-review.

[fivetran-joemarkiewicz]

I think we discussed during that quick show and tell that it may be better to use the following. This way you must be using a dictionary instead of not string. This way if customers need to pass an int or something, they aren't hit with an error.

Suggested change

	{%- if property is not string -%}
	{%- if property is mapping -%}

[fivetran-reneeli]

Oh yes that's right-- testing it now

[fivetran-reneeli]

Test with the following packages.yml in dbt_stripe_source

packages:
# - package: fivetran/fivetran_utils
#   version: [">=0.4.0", "<0.5.0"]
- package: dbt-labs/spark_utils
  version: [">=0.3.0", "<0.4.0"]

- git: https://github.com/fivetran/dbt_fivetran_utils.git
  revision: bugfix/json_replace
  warn-unpinned: false

[fivetran-reneeli]

And adding this to the project yml:

    stripe__plan_metadata:
        - name: limit
          alias: limit_z
        - name: feature.id
          alias: feature_id
        - name: plan.handle
          alias: plan_handle
        - name: set
          alias: sets
        - name: 123
          alias: onetwothree

[fivetran-reneeli]

Hi @fivetran-joemarkiewicz , I made the updates and also updated the changelog and readmes and versioning. Lmk if it looks good!

[fivetran-joemarkiewicz]

@fivetran-reneeli just a few small comments regarding the versioning of the package and folding the changelog entries into the soon to be released 0.4.1 entry.

Let me know once the updates have been applied and I can re-review

[fivetran-joemarkiewicz]

0.4.2 hasn't been released yet. We should fold these into the 0.4.1 changelog entries instead.

[fivetran-reneeli]

updated

[fivetran-joemarkiewicz]

0.4.1 isn't released yet, we can keep this as 0.4.1

[fivetran-reneeli]

updated

[fivetran-joemarkiewicz]

Same as above, we can keep as 0.4.1

[fivetran-reneeli]

updated

[fivetran-joemarkiewicz]

I just have one small request to update the CHANGELOG. Once you commit that suggestion, this will be good to merge into releases/v0.4.latest