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

Extend unit creation pipeline mode to snowflake #1691

Merged
merged 5 commits into from Sep 29, 2023

Conversation

lukesonnet
Copy link
Contributor

Features and Changes

Allows units table creation for Snowflake integration.

Required some tweaks to our ability to create table paths.

Testing

Works in localhost connected to my snowflake.

Permissions are a little wonky so will need good docs here.

@github-actions
Copy link

github-actions bot commented Sep 27, 2023

Your preview environment pr-1691-bttf has been deployed.

Preview environment endpoints are available at:

}

// Add schema if required
if (this.requiresSchema) {
if (this.requiresSchema || requireSchema) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not blocking, but perhaps there is a better variable name for requireSchema. Maybe queryRequiresSchema - it took me a minute to understand the different between requireSchema and this.requiresSchema.

Copy link
Collaborator

@mknowlton89 mknowlton89 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - Left one comment/suggestion on a variable name.

I tested this with some of my existing data sources (GA4, Segment, Snowflake) and I didn't run into any issues with creating the datasource, building the information_schema, or building any automatic metrics (only GA4 & Segment support that, currently).

I don't know how to test the net-new logic around the pipeline - if you'd like me to test it, just lmk and we can sync up on steps to do so.

@lukesonnet
Copy link
Contributor Author

As per offline discussion, the quotes aren't necessary, so I removed that and took your other advice.

Re-tested and everything is working for snowflake info schemas and creating tables (on my end).

@lukesonnet lukesonnet marked this pull request as ready for review September 28, 2023 18:34
Copy link
Collaborator

@mknowlton89 mknowlton89 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@lukesonnet lukesonnet merged commit eeef29d into main Sep 29, 2023
3 checks passed
@lukesonnet lukesonnet deleted the ls/create_units_snowflake branch September 29, 2023 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants