-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
🐛 Source Mixpanel - Add new datatime formats for state for cohort_members stream, added obsolete state reset for cohort_members stream #38066
Conversation
… obsolete state reset for cohort_members stream
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
… into midavadim/mixpanel-datatime-fix
airbyte-integrations/connectors/source-mixpanel/source_mixpanel/manifest.yaml
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left a few comments. main questions
- can semi-incremental be released separately from the bug fixes?
- can the filters only rely on stream_interval instead of stream_state?
airbyte-integrations/connectors/source-mixpanel/source_mixpanel/manifest.yaml
Outdated
Show resolved
Hide resolved
airbyte-integrations/connectors/source-mixpanel/source_mixpanel/components.py
Show resolved
Hide resolved
airbyte-integrations/connectors/source-mixpanel/source_mixpanel/manifest.yaml
Show resolved
Hide resolved
airbyte-integrations/connectors/source-mixpanel/source_mixpanel/manifest.yaml
Outdated
Show resolved
Hide resolved
airbyte-integrations/connectors/source-mixpanel/source_mixpanel/manifest.yaml
Outdated
Show resolved
Hide resolved
…rval instead of stream_slice + stream_state
|
# Conflicts: # airbyte-integrations/connectors/source-mixpanel/metadata.yaml # airbyte-integrations/connectors/source-mixpanel/pyproject.toml # docs/integrations/sources/mixpanel.md
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good thanks @midavadim !
releaseStage: generally_available | ||
releases: | ||
breakingChanges: | ||
3.0.0: | ||
message: | ||
In this release, state for CohortMembers is changed to per partition format. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lazebnyi @midavadim don't we need to have impactscope?
-let's change this message to be more customer friendly. This information is good, but I think it belongs in the migration guide, not the breaking change warning.
-please pic a format for the stream either CohortMembers or cohort_members
-user needs to understand what action needs to be taken. General format is "x stream has changed for y reason. Please reset x stream. See migration guide for further details."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
In this release, we introduce breaking change for `CohortMembers` stream: | ||
- State is changed to per-partition format. | ||
- Key is changed to correct unique key (based on 'distinct_id' and 'cohort_id' fields) since previous key was not unique and didn't support possibility for user be in a few different cohorts. | ||
Semi-incremental `Cohorts`, `CohortMembers` and `Engage` streams with client-side filtering extract records since user provided or default (1 year old) start_date. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand what you are saying here. Can you explain it a different way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
Co-authored-by: katmarkham <40400595+katmarkham@users.noreply.github.com>
Co-authored-by: katmarkham <40400595+katmarkham@users.noreply.github.com>
What
ERROR # 1 cohorts stream
Error 2 - cohort_members stream
Error 3 cohort_members::
Problem 4:
Fixing non-unique primary key for cohort_members stream
#37833
Old key: distinct_id
New key: distinct_id, cohort_id
How
Review guide
User Impact
Breaking change for
CohortMembers
stream:Can this PR be safely reverted and rolled back?