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.

Sign up for GitHub

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

Jump to bottom

Remove CDC from EVENTS migration group #6804

Merged
merged 5 commits into from
Jan 22, 2025
Merged

Remove CDC from EVENTS migration group #6804

merged 5 commits into from
Jan 22, 2025

Conversation

volokluev
Copy link
Member

@volokluev volokluev requested a review from a team as a code owner January 22, 2025 21:07
@MeredithAnya
Copy link
Member

wanna also remove the references in the settings files like

snuba/snuba/settings/__init__.py

Line 100 in caddadc

"cdc",

@volokluev volokluev enabled auto-merge (squash) January 22, 2025 21:08
@codecov Codecov
Copy link

codecov bot commented Jan 22, 2025 •
edited
Loading

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
1212 1 1211 9
View the top 1 failed tests by shortest run time 
tests.migrations.test_legacy_use::test_legacy_use
Stack Traces | 0.247s run time 
Traceback (most recent call last):
  File ".../tests/migrations/test_legacy_use.py", line 43, in test_legacy_use
    pytest.fail(
  File ".../local/lib/python3.11.../site-packages/_pytest/outcomes.py", line 196, in fail
    raise Failed(msg=reason, pytrace=pytrace)
Failed: Migration 0001_migrations in group MigrationGroup.CDC is using legacy types

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

ayirr7
ayirr7 reviewed Jan 22, 2025
View reviewed changes
snuba/migrations/groups.py Outdated
@@ -82,12 +83,16 @@ def __init__(
storage_sets_keys={StorageSetKey.MIGRATIONS},
readiness_state=ReadinessState.COMPLETE,
),
MigrationGroup.CDC: _MigrationGroup(
loader=SystemLoader(),
Copy link
Member

Choose a reason for hiding this comment

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

We might need to define a different loader here

Copy link
Member Author

Choose a reason for hiding this comment

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

you're so right bestie

@volokluev volokluev mentioned this pull request Jan 22, 2025
Open
@volokluev volokluev merged commit 0b28dd9 into master Jan 22, 2025
31 checks passed
@volokluev volokluev deleted the volo/no_cdc branch January 22, 2025 22:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ayirr7 ayirr7 ayirr7 approved these changes

Assignees
No one assigned
Labels
migrations
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@volokluev @MeredithAnya @ayirr7