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): DLT + temporal #18700

Merged
merged 74 commits into from
Nov 29, 2023
Merged

feat(data-warehouse): DLT + temporal #18700

merged 74 commits into from
Nov 29, 2023

Conversation

EDsCODE
Copy link
Member

@EDsCODE EDsCODE commented Nov 17, 2023

Problem

  • can't sync data

Changes

  • Implements a temporal workflow that allows for stripe data to be synced

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

How did you test this code?

@EDsCODE EDsCODE changed the base branch from dw-streams to master November 21, 2023 21:54
latest_migrations.manifest Outdated Show resolved Hide resolved
@EDsCODE EDsCODE changed the base branch from master to dw-refactor-temporal-env-var November 22, 2023 19:13
@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 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

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

  • chromium: 0 added, 1 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.

@@ -300,3 +300,5 @@ class FlagRequestType(str, Enum):


ENRICHED_DASHBOARD_INSIGHT_IDENTIFIER = "Feature Viewed"
DATA_WAREHOUSE_TASK_QUEUE = "data-warehouse-task-queue"
BATCH_EXPORTS_TASK_QUEUE = "no-sandbox-python-django"
Copy link
Contributor

Choose a reason for hiding this comment

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

mental note: I should change the batch exports queue name.

Copy link
Contributor

Choose a reason for hiding this comment

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

new mental note: Doing this would require being really careful, so maybe I'll get over the fact we have a bad name for this queue 💀

@EDsCODE
Copy link
Member Author

EDsCODE commented Nov 28, 2023

Note: migration test failure should be irrelevant...seems like dependency is not updating which has the field that test says is missing

Yeah, this will be fine. The migration check is using packages from master to validate but this PR introduces a package update that the latest migration relies on

Base automatically changed from dw-refactor-temporal-env-var to master November 29, 2023 14:32
@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

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

  • chromium: 0 added, 1 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.

@EDsCODE EDsCODE merged commit 2a6a13c into master Nov 29, 2023
77 checks passed
@EDsCODE EDsCODE deleted the dw-sync-experiment branch November 29, 2023 19:05
Copy link

sentry-io bot commented Nov 30, 2023

Suspect Issues

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

  • ‼️ SchemaValidationError: Schema validation failed posthog.warehouse.data_load.sync_table in is_sc... View Issue
  • ‼️ ContainerInjectableContextMangled: When restoring context ConfigSectionContext, instance <dlt.common.configuration.specs.config_sect... dlt.common.configuration.container in injectabl... 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.

3 participants