Velir / dbt-ga4

dbt Package for modeling raw data exported by Google Analytics 4. BigQuery support, only.
MIT License
312 stars 134 forks source link

Errors with version `4.0.0` and later commits #262

Closed zhyatt closed 1 year ago

zhyatt commented 1 year ago

I have been trying to get newer versions working, but after having success with some initial runs I have begun to run into errors. With the latest version release (version: [">=4.0.0", "<4.1.0"]) I ran into the _dbt_max_partition issue as noted in https://github.com/Velir/dbt-ga4/issues/253. I see that issue happening up to and including commit af6b3815977db5462c2c2888c6f5bcfa31cb5284.

But then when I use other commits on the main branch per the suggestion, such as fd26c3c35ee1b473fa87ab531c9a1a07d862f560, I get a different error with a stack trace that isn't too helpful, it ends in this:

TypeError: 'NoneType' object cannot be interpreted as an integer

I suspected this could be related to my configuration and realized I had the old frequency variable value but didn't have the static_incremental_days variable set, so I added that, but since we don't have streaming enabled, we get this error:

xxxxxxxxxx:analytics_#########.events_intraday_* does not match any table.

Does this package require streaming to be enabled to use? Or was this an issue introduced sometime after version 4.0.0 that accidentally required streaming? Hopefully I didn't miss something simple in the documentation that could be causing this.

adamribaudo-velir commented 1 year ago

I just merged #257 which may resolve your issue. I'm pretty certain this is on us and not you :) I'll get a release out soon as I'm sure it's causing some confusion.

Can you try installing from the main branch and letting me know the results? Add the following to packages.yml:

packages:
  - git: "https://github.com/Velir/dbt-ga4.git"
adamribaudo-velir commented 1 year ago

And yes, static_incremental_days is now a required variable (docs have been updated). If anyone knows how to produce a meaningful error message to users when that variable is missing, I'd love to hear it.

dgitis commented 1 year ago

https://docs.getdbt.com/reference/dbt-jinja-functions/exceptions

{% if static_incremental_days is none %}
  {{ exceptions.raise_compiler_error("Missing required static_incremental_days variable ") }}
{% endif %}
{% if static_incremental_days is NaN %}
  {{ exceptions.raise_compiler_error("Type error: static_incremental_days must be an integer ") }}
{% endif %}

I'm not sure about the NaN check.

Also, test for '0' being valid since that evaluates to false. I used static_incremental_days > -1 in one check that I did.

adamribaudo-velir commented 1 year ago

@dgitis not sure what's up but that code doesn't seem to work. I implemented it at the top of base_ga4__events and I still get:

11:05:21  Encountered an error:
'NoneType' object cannot be interpreted as an integer

Also tried:

{% if var('static_incremental_days', false) == false %}
  {{ exceptions.raise_compiler_error("Missing required variable: static_incremental_days") }}
{% endif %}
dgitis commented 1 year ago

It looks like the compiler works on macros and materializations. Maybe we can create a partitioning macro. I tried and had trouble getting variables to exist in both macro and project scope, but that could be a knowledge issue.

zhyatt commented 1 year ago

Can you try installing from the main branch and letting me know the results?

I updated the package to version: [">=5.0.0", "<5.1.0"] and this is now working with the static_incremental_days variable set. Thanks for the quick fix and all your hard work on this package!

dgitis commented 1 year ago

@adamribaudo-velir , I'll take a stab at moving partitioning into a macro and then checking for static_incremental_days and throwing exceptions there on the weekend. I've got a POC on a fork that I can work off of.

adamribaudo-velir commented 1 year ago

Sounds like this is working for @zhyatt . Feel free to open a new issue or PR related to exception messages, @dgitis