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

feat(data-warehouse): Added a snowflake integration #22723

Merged
merged 17 commits into from
Jun 6, 2024
Merged

Conversation

Gilbert09
Copy link
Member

Changes

  • Added a snowflake source integration to data warehouse
  • Works the same as the postgres source - uses the same source connector, just with a different connection string
  • Downgrades sqlalchemy due to lack of support for newer versions in snowflake-sqlalchemy package
    • I'm watching this issue for updates on when they'll support sqlalchemy v2+ - looking like Q4 2024 at this point (mad, I know).
  • Updates DLT package and uses a DLT helper class to normalize schema names when building the S3 URL
    • This is to stop any mismatch happening between where DLT loads the parquet files and our references to them

image

Does this work well for both Cloud and self-hosted?

Yes

How did you test this code?

  • Local testing, didn't feel the need for unit tests due to the same backend/source as postgres is being used

@Gilbert09 Gilbert09 requested a review from EDsCODE June 5, 2024 16:38
Copy link
Contributor

github-actions bot commented Jun 5, 2024

Size Change: 0 B

Total Size: 1.06 MB

ℹ️ View Unchanged
Filename Size
frontend/dist/toolbar.js 1.06 MB

compressed-size-action

Comment on lines 153 to 159
update_inputs = UpdateExternalDataJobStatusInputs(
id=run_id, run_id=run_id, status=ExternalDataJob.Status.COMPLETED, latest_error=None, team_id=inputs.team_id
id=run_id,
run_id=run_id,
status=ExternalDataJob.Status.COMPLETED,
latest_error=None,
team_id=inputs.team_id,
)
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Should we put this in a finally to also update the status to FAILED in case an exception is raised?

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

  • chromium: 0 added, 2 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

  • chromium: 0 added, 2 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

3 snapshot changes in total. 0 added, 3 modified, 0 deleted:

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@Gilbert09 Gilbert09 merged commit a11be6e into master Jun 6, 2024
84 checks passed
@Gilbert09 Gilbert09 deleted the tom/snowflake-sw branch June 6, 2024 14:39
Copy link

sentry-io bot commented Jun 6, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ PipelineStepFailed: Pipeline execution failed at stage extract when processing package 1718042620.437888 with exception: posthog.temporal.data_imports.pipelines.pipelin... View Issue
  • ‼️ ActivityError: Activity task failed posthog.temporal.data_imports.external_data_job... View Issue
  • ‼️ PipelineStepFailed: Pipeline execution failed at stage extract when processing package 1717992953.9715526 with except... posthog.temporal.data_imports.pipelines.pipelin... View Issue
  • ‼️ AttributeError: 'NoneType' object has no attribute 'row_counts' posthog.temporal.data_imports.pipelines.pipelin... View Issue
  • ‼️ ExternalDataJob.DoesNotExist: ExternalDataJob matching query does not exist. posthog.warehouse.models.external_data_job in g... View Issue

Did you find this useful? React with a 👍 or 👎

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.

4 participants