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

Implementing multi-project architecture ( issue #231 ) #243

Closed

Conversation

jice-lavocat
Copy link

Description & motivation

Updating the dbt_projects.yml format to specify the GCP project and GA4 property_id.

Use case: when sources are spread on different distinct GCP Projects.
Related to #231

Checklist

  • I have verified that these changes work locally
  • I have updated the README.md (if applicable)
  • I have added tests & descriptions to my models (and macros if applicable)
  • I have run dbt test and python -m pytest . to validate existing tests

@jice-lavocat jice-lavocat marked this pull request as ready for review June 28, 2023 07:05
@jice-lavocat jice-lavocat changed the title Multiple property settings Implementing multi-project architecture ( issue #231 ) Jun 28, 2023
@jice-lavocat
Copy link
Author

jice-lavocat commented Jun 28, 2023

@adamribaudo-velir : I somehow messed up while fixing the merge conflict of PR235.
You will find here a new PR with the previous code, adapted to the newest version of your code.

@adamribaudo-velir
Copy link
Collaborator

@jice-lavocat sorry, I was under the impression that your code would still work correctly if users left the existing variables in place in the format of:

property_ids: [11111111,222222222]

When I use that configuration, I get a compilation error as follows:

Compilation Error in model base_ga4__events (models\staging\base\base_ga4__events.sql)
  argument of type 'int' is not iterable

  > in macro combine_property_data (macros\combine_property_data.sql)
  > called by model base_ga4__events (models\staging\base\base_ga4__events.sql)

Presumably due to this line which would be searching for a dictionary key of 'project' inside the first element of the array which is an integer:

{% if 'project' not in var('property_ids')[0] %}

Could you find a way to handle both types of configuration? single project and multi-project?

@hugocaillol
Copy link

hugocaillol commented Dec 22, 2023

Hello !

I started using the Velir package recently and had a bit of an issue with not being able to specify two distinct projects for reading data (the {{var('project')}}.analytics_{{property_id}}.events_intraday_{{relation_suffix}} part) and writing data ( the {{var('project')}}.{{var('dataset')}}.events_intraday_{{relation_suffix}}{{property_item['property_id']}} ) ie I would also like to have the combined data stored in a different project than the one where we keep our data sources (it led to bigquery errors because of the user (me) not having the rights to write in the ingestion project which can be worked around but still isn't perfect for us)

So I looked a bit around and found this pr with some changes I would love to have and was wondering if it had a chance to be implemented in the near future or if I should better accept to make changes locally and make my own custom version of the velir package

@adamribaudo-velir
Copy link
Collaborator

I agree we should take another look at this. There will likely be some breaking changes to update the variable names to reflect the true intention (ex: source_project and source_dataset instead of project and dataset). I can't commit to a timeline, though.

@hugocaillol
Copy link

Hello Adam, thank for the answer, I'll keep for updates then

@adamribaudo-velir
Copy link
Collaborator

adamribaudo-velir commented Dec 29, 2023

I went ahead and made this updates :) @hugocaillol

Created a new PR so I could start from scratch. See the notes in the PR for the variable name changes and let me know how it works.

@adamribaudo-velir
Copy link
Collaborator

I believe the intention of this PR is contained in #294 which has been merged. Please let me know if you feel otherwise, @jice-lavocat . Closing PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants