fix(aci): Implement DataSource.normalize_before_relocation_import #103002
Conversation
This file contains hidden or 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
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #103002 +/- ##
===========================================
+ Coverage 80.58% 80.83% +0.24%
===========================================
Files 8937 8950 +13
Lines 391228 392336 +1108
Branches 24887 24887
===========================================
+ Hits 315277 317136 +1859
+ Misses 75581 74830 -751
Partials 370 370 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
cathteng
reviewed
Nov 11, 2025
Comment on lines
+33
to
+34
| # DataSource.source_id dynamically references different models based on the 'type' field. | ||
| # We declare all possible dependencies here to ensure proper import ordering. |
Member
There was a problem hiding this comment.
Does the order matter because we need the pk mappings for the other tables to exist before this table is relocated?
Comment on lines
81
to
82
| # Referenced model not in pk_map. This may be correct (reset_pks=False) or broken | ||
| # (reset_pks=True but referenced model was filtered out or failed to import). |
Member
There was a problem hiding this comment.
Should we return None if we can't map the source_id? I think with this logic it will keep this object mapped to the old source_id, which could be inaccurate?
Member
Author
There was a problem hiding this comment.
We should, and now we do. I got confused by test logic, but now know better.
cathteng
approved these changes
Nov 12, 2025
Comment on lines
+741
to
+743
| DataSource.source_id is a dynamic foreign key that gets remapped during import via the | ||
| normalize_before_relocation_import method. Since the remapping is handled there, we just | ||
| need to verify that both sides have a valid source_id value, without comparing the actual values. |
andrewshie-sentry
pushed a commit
that referenced
this pull request
Nov 13, 2025
…03002) To be properly imported from backup (as with self-host to saas migration) DataSource needs to remap ids to new values. This involves tracking dependencies and translating in normalize_before_relocation_import. The field comparator has to be somewhat lax here, as the actual foreign item referenced is dependent on two fields, which isn't really a pattern supported by our ForeignKey comparator infrastructure. For now, at least, we assume the fields being referenced are valid (a big improvement over the current prod code, where we have broken id references and also ignore that), but this will be revisited in a follow-up.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 be properly imported from backup (as with self-host to saas migration) DataSource needs to remap ids to new values.
This involves tracking dependencies and translating in normalize_before_relocation_import.
The field comparator has to be somewhat lax here, as the actual foreign item referenced is dependent on two fields, which isn't really a pattern supported by our ForeignKey comparator infrastructure. For now, at least, we assume the fields being referenced are valid (a big improvement over the current prod code, where we have broken id references and also ignore that), but this will be revisited in a follow-up.