feat(backup): Generate cloned QuerySubscription on import #56023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
Scope: Backend
azaslavsky
force-pushed
the
azaslavsky/backup/org_auth_token
branch
from
dc76ccf
to
435bdf3
Compare
azaslavsky
force-pushed
the
azaslavsky/backup/query_subscription
branch
from
80b5447
to
183f191
Compare
azaslavsky
force-pushed
the
azaslavsky/backup/org_auth_token
branch
from
435bdf3
to
4577593
Compare
azaslavsky
force-pushed
the
azaslavsky/backup/query_subscription
branch
from
183f191
to
c92fc3c
Compare
azaslavsky
force-pushed
the
azaslavsky/backup/org_auth_token
branch
from
4577593
to
dfb047a
Compare
azaslavsky
force-pushed
the
azaslavsky/backup/query_subscription
branch
from
c92fc3c
to
010e70e
Compare
Codecov Report
@@ Coverage Diff @@
## master #56023 +/- ##
===========================================
+ Coverage 66.67% 79.99% +13.32%
===========================================
Files 5063 5066 +3
Lines 217573 218017 +444
Branches 36828 36898 +70
===========================================
+ Hits 145061 174398 +29337
+ Misses 66929 38270 -28659
+ Partials 5583 5349 -234
|
src/sentry/snuba/models.py
Outdated
|
|
||
| # Keep the original dates. | ||
| subscription.date_added = self.date_added | ||
| subscription.date_updated = self.date_updated |
There was a problem hiding this comment.
Does it make sense to have a new date_updated here instead of using the old value?
There was a problem hiding this comment.
You're right - I've removed this line.
| if old_pk is None: | ||
| return None | ||
|
|
||
| subscription = create_snuba_subscription(self.project, self.type, self.snuba_query) |
There was a problem hiding this comment.
Previously, were imports for this working at all? Wondering if there was some setup missing that create_snuba_subscription does
There was a problem hiding this comment.
I don't see how they would have worked. This is one of the (many) ways that old import/export was broken.
To avoid creating duplicate `subscription_id`s, rather than naively copying in imported model data, we instead take this data to construct a new call to `create_snuba_subscription`. This will generate an identical subscription (modulo creation/edit dates, which we overwrite), but with a new `subscription_id`. Closes getsentry/team-ospo#193
azaslavsky
force-pushed
the
azaslavsky/backup/query_subscription
branch
from
010e70e
to
c4e56c6
Compare
hubertdeng123
approved these changes
Sep 14, 2023
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
To avoid creating duplicate
subscription_ids, rather than naively copying in imported model data, we instead take this data to construct a new call tocreate_snuba_subscription. This will generate an identical subscription (modulo creation/edit dates, which we overwrite), but with a newsubscription_id.Closes getsentry/team-ospo#193