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

customers missing when running package on test data #19

Closed
leinemann opened this issue Jun 30, 2021 · 15 comments
Closed

customers missing when running package on test data #19

leinemann opened this issue Jun 30, 2021 · 15 comments
Assignees
Labels
enhancement New feature or request

Comments

@leinemann
Copy link

Hi there, I would like to run this package on our stripe prod data aswell as the test data (using the fivetran test connector). I can override schema names accordingly. But test customers will never be pulled in because their livemode flag will be set to false. Would it be worth adding some configuration option to enable to run this package on test data?
Just wondering if there are some considerations that I'm not aware of?
many thanks,
Christoph.

@fivetran-joemarkiewicz
Copy link
Contributor

Hi @leinemann thanks for opening this issue!

The configuration could for sure exist to enable customer test data to run on the package. I am curious what the broader use case of using the test data is for? I am hesitant to add further configuration (especially as there is already more than I would like), but if it makes sense to apply this update for all customers then we can move forward with this feature request.

Thanks!

@fivetran-jamie fivetran-jamie mentioned this issue Jul 9, 2021
12 tasks
@leinemann
Copy link
Author

Thank you for looking into this! I wanted to share some context on the requirement. Our business wants the test customers shown in a parallel but very same way as the real customers. So for our setup we use the fivetran _test connector to get the test accounts (which all are livemode false) and then we apply this dbt package to get them into a _test analytics schema. This allows a segregated but yet like-for-like capture of test vs real customers all the way through the reporting.

@fivetran-jamie
Copy link
Contributor

@leinemann i see -- so an ideal solution for you here would be to have the ability to run the package on only test customers vs only not-test customers, is that correct? in the PR i drafted, i made it so you can run on only not-test customers vs test AND not-test customers, which doesn't sound like what you're looking for

@leinemann
Copy link
Author

Yes, I see it as the continuation of the ’Stripe test mode’ connector. It writes test data into our strip_test schema and equally I would like to run the stripe dbt packages to take this test data and create the analytics views with test data only which means that it needs to run also picking up livemode being false everywhere. So this means I'm running my strip dbt packages twice, the second time with stripe_test being the source.
There might be better ways of creating the test-only analytics views - please let me know.

@fivetran-jamie
Copy link
Contributor

gotcha, sounds like we're on the same page then. curious -- when you run the package the second time on your test-only data, are there any other configurations you have to make to do so? like writing to a different schema, etc

@fivetran-jamie
Copy link
Contributor

@leinemann this might be good to connect over in office hours! i'll be out next week but @fivetran-joemarkiewicz will be here 5tran.co/office-hours

@leinemann
Copy link
Author

sorry about the late reply - yes, the second run for test data we run with some var overrides for the source and target schemata for the src and the other stripe package. The office hour link returns a 404 - could you please re-paste?

@fivetran-joemarkiewicz
Copy link
Contributor

Our apologies here is the proper link.

As @fivetran-jamie is OOO this week I will be helping close this Issue out. I think I am understanding @leinemann that you would be happy with a variable that simply only brings in test customers when it is configured. We can very well do that!

I did want to confirm with you that you only want the customer object to be filtered for test data, correct? I am aware that many other objects (charge, invoice, invoice_line, payment_intent, etc.) have a livemode field. Would you want only test data for these tables as well, or just customers?

Happy to chat more over office hours

@leinemann
Copy link
Author

Hi @fivetran-joemarkiewicz, the customer object was were we first noticed the challenge but I think it would make sense to have a property applied across the board. Let me schedule an office hour slot for the end of the week as I'm OOO as well at the moment.
Thanks!

@fivetran-jamie
Copy link
Contributor

hey @leinemann if you're still down to do your own PR lmk if you need any help! hoping to release this before our sprint ends next wednesday

@leinemann
Copy link
Author

Hey @fivetran-jamie! Yes, ok, great! I've forked the repo, created a config_livemode_cl branch, rebased master and added other places where I think we'll need to consider livemode. I believe the structure is already so nice that the change will be limited to tmp models? Please let me know if I'm mistaken here. Could you let me know if this WIP PR leinemann#1 against my own master is going in the right direction and if so we can finalise the naming other reviews and formalities and I can raise a proper PR against the main repo?

@fivetran-jamie
Copy link
Contributor

fivetran-jamie commented Aug 9, 2021

@leinemann looks great! i would maybe suggest renaming the test-data boolean variable from test_data_only to using_ test_data_only or something, just to keep in line with our naming conventions. other than that, looks good to move forward and make a PR against main

@leinemann
Copy link
Author

ok, thanks @fivetran-jamie, what do you think about calling it what it is i.e. using_livemode and inverting the boolean. i.e. defaulting it to true with the option of setting it to false?

@fivetran-jamie
Copy link
Contributor

wahoo just pushed version 0.4.1 of the package, which includes your code @leinemann. thanks for opening the issue (and solving it 😄 )

@leinemann
Copy link
Author

😀 thanks for your support!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants