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(batch-exports): Add Redshift to BatchExport destinations #18059

Merged
merged 18 commits into from
Nov 1, 2023

Conversation

tomasfarias
Copy link
Contributor

@tomasfarias tomasfarias commented Oct 18, 2023

Problem

Users of the PostHog Redshift Export plugin need to be migrated over to PostHog batch exports to take advantage of the features provided by the new platform, and to be in line with the rest of the export destinations.

Changes

  1. Abstract insert activity execution into a function
    Not necessary for this particular issue, but I got tired of copying and pasting Workflow code. I'll follow-up with a refactor to use the function in all other workflows.

  2. Add a RedshiftBatchExportWorkflow
    This new Workflow simply calls the Postgres activity but with Redshift-specific parameters. Since we cannot do COPY on Redshift without breaking compatibility and extra complexity, we do not use the Postgres activity anymore, but instead run an INSERT query. We do re-use as much as we can from Postgres.

  3. Add Redshift to BatchExport destinations
    Model updates to support the new destination

TODO:

  1. Frontend support for Redshift
  2. Testing

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

How did you test this code?

Locally ran unit tests:

❯ REDSHIFT_USER=user REDSHIFT_HOST=host REDSHIFT_PASSWORD=password DEBUG=1 pytest posthog/temporal/tests/batch_exports/test_redshift_batch_export_workflow.py -vv
==================================== test session starts =====================================
platform darwin -- Python 3.10.11, pytest-7.4.0, pluggy-0.13.1 -- src/github.com/PostHog/posthog/.direnv/python-3.10.11/bin/python
cachedir: .pytest_cache
django: settings: posthog.settings (from ini)
rootdir: src/github.com/PostHog/posthog
configfile: pytest.ini
plugins: icdiff-0.6, split-0.8.1, flaky-3.7.0, cov-4.1.0, asyncio-0.21.1, mock-3.11.1, Faker-17.5.0, syrupy-1.7.4, django-4.5.2, env-0.8.2
asyncio: mode=strict
collected 3 items                                                                            

posthog/temporal/tests/batch_exports/test_redshift_batch_export_workflow.py::test_insert_into_redshift_activity_inserts_data_into_redshift_table PASSED [ 33%]
posthog/temporal/tests/batch_exports/test_redshift_batch_export_workflow.py::test_redshift_export_workflow[hour] PASSED [ 66%]
posthog/temporal/tests/batch_exports/test_redshift_batch_export_workflow.py::test_redshift_export_workflow[day] PASSED [100%]

---------------------------------- snapshot report summary -----------------------------------

===================================== 3 passed in 51.14s =====================================

@tomasfarias tomasfarias force-pushed the feat/redshift-destination-for-batch-exports branch from dce972a to a9394ed Compare October 19, 2023 10:42
Base automatically changed from feat/backfill-abstraction to master October 19, 2023 13:09
@tomasfarias tomasfarias force-pushed the feat/redshift-destination-for-batch-exports branch from a9394ed to 4961f88 Compare October 20, 2023 00:25
@tomasfarias tomasfarias marked this pull request as ready for review October 20, 2023 09:59
@tomasfarias tomasfarias mentioned this pull request Oct 20, 2023
36 tasks
@posthog-bot
Copy link
Contributor

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week.

@tomasfarias tomasfarias removed the stale label Oct 30, 2023
@tomasfarias tomasfarias force-pushed the feat/redshift-destination-for-batch-exports branch 5 times, most recently from 50add8f to 6e08239 Compare November 1, 2023 13:37
@tomasfarias tomasfarias force-pushed the feat/redshift-destination-for-batch-exports branch from 6e08239 to 16f0267 Compare November 1, 2023 16:44
@tomasfarias tomasfarias enabled auto-merge (squash) November 1, 2023 17:27
@tomasfarias tomasfarias merged commit 2adc2f9 into master Nov 1, 2023
72 of 73 checks passed
@tomasfarias tomasfarias deleted the feat/redshift-destination-for-batch-exports branch November 1, 2023 17:33
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