Skip to content
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

[Bug] Missing column in intermediate model if variable is set #102

Closed
2 of 4 tasks
antoon-r opened this issue Apr 14, 2023 · 3 comments · Fixed by #105
Closed
2 of 4 tasks

[Bug] Missing column in intermediate model if variable is set #102

antoon-r opened this issue Apr 14, 2023 · 3 comments · Fixed by #105
Assignees
Labels
priority:p4 Affects few users; pick up when available status:in_progress Currently being worked on type:bug Something is broken or incorrect update_type:models Primary focus requires model updates

Comments

@antoon-r
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Describe the issue

When running project with var hubspot_email_event_status_change_enabled: false, it fails with Database Error

Problem is on this line:

{% if fivetran_utils.enabled_vars(['hubspot_email_event_status_change_enabled']) %}

When column unsubscribes is not created, it should not be accessed, too. It can be fixed by using custom email_metrics array in dbt_project.yml, but it is impossible to figure it out without looking into the code of package

Relevant error log or model output

09:39:53  Database Error in model hubspot__email_campaigns (models/marketing/hubspot__email_campaigns.sql)
09:39:53    Name unsubscribes not found inside email_sends at [65:25]
09:39:53    compiled Code at target/run/hubspot/models/marketing/hubspot__email_campaigns.sql
09:39:53  
09:39:53  Database Error in model int_hubspot__email_metrics__by_contact_list (models/marketing/intermediate/int_hubspot__email_metrics__by_contact_list.sql)
09:39:53    Unrecognized name: unsubscribes at [74:13]
09:39:53    compiled Code at target/run/hubspot/models/marketing/intermediate/int_hubspot__email_metrics__by_contact_list.sql
09:39:53  
09:39:53  Database Error in model hubspot__contacts (models/marketing/hubspot__contacts.sql)
09:39:53    Unrecognized name: unsubscribes at [112:13]
09:39:53    compiled Code at target/run/hubspot/models/marketing/hubspot__contacts.sql

Expected behavior

Run does not fail with Database Error

dbt Project configurations

vars:
hubspot_email_event_status_change_enabled: false

Package versions

packages:

  • package: fivetran/hubspot
    version: 0.9.0

What database are you using dbt with?

bigquery

dbt Version

1.3.1

Additional Context

No response

Are you willing to open a PR to help address this issue?

  • Yes.
  • Yes, but I will need assistance and will schedule time during our office hours for guidance
  • No.
@fivetran-joemarkiewicz fivetran-joemarkiewicz added the status:scoping Currently being scoped label Apr 18, 2023
@fivetran-joemarkiewicz
Copy link
Contributor

HI @antoon-r thanks for opening this bug report. I agree with you that these downstream models fail when the hubspot_email_event_status_change_enabled variable is set to false, but the other variables are enabled. I believe this may have been missed because we may have not come across a user who has email_event_status_change turned off, but has all other email events turned on. Regardless, this is something we will want to address within the package.

Upon further scoping I found the following additional problems areas:

  • We define the email metrics in the dbt_project.yml via the email_metrics variable
    • Note that unsubscribes is included by default
  • In hubspot__email_campaign we are iterating over the metrics from the above variable. However, as you identified, unsibscribes does not exist if the hubspot_email_event_status_change_enabled variable is set to false.
  • Similarly in hubspot__contacts we are doing the same iteration operation
  • Finally, we can see in int_hubspot__email_metrics__by_contact_list we are doing the exact same operation.

Based on the above, it is clear that we should not be iterating on a field that could possibly not exist in the data based if a variable is set to false. I believe what may be the optimal path forward is to add the functionality to the package to dynamically infer the metrics. If the metric doesn't exist, then it can be skipped.

Nevertheless, we will add this to our upcoming sprint to be addressed. Thanks again for raising this to our team!

@fivetran-joemarkiewicz fivetran-joemarkiewicz added type:bug Something is broken or incorrect priority:p4 Affects few users; pick up when available status:accepted Scoped and accepted into queue update_type:models Primary focus requires model updates and removed status:scoping Currently being scoped labels Apr 18, 2023
@fivetran-catfritz fivetran-catfritz self-assigned this Apr 26, 2023
@fivetran-catfritz fivetran-catfritz added status:in_progress Currently being worked on and removed status:accepted Scoped and accepted into queue labels Apr 26, 2023
@fivetran-catfritz fivetran-catfritz linked a pull request Apr 28, 2023 that will close this issue
15 tasks
@fivetran-catfritz
Copy link
Contributor

Hi @antoon-r I've created a test branch for you to try out. To use it, simply replace the dbt_hubspot section in your packages.yml with the following:

- git: https://github.com/fivetran/dbt_hubspot.git
  revision: bug/email-metrics-variable
  warn-unpinned: false

You should be able to use this new branch without any further modifications. Please let me know how it works for you!

@antoon-r
Copy link
Author

antoon-r commented May 1, 2023

Hi @fivetran-catfritz this branch fixes the problem, thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority:p4 Affects few users; pick up when available status:in_progress Currently being worked on type:bug Something is broken or incorrect update_type:models Primary focus requires model updates
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants