Skip to content

feat(backup): Add ForeignKeyComparator #54558

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

Merged
merged 1 commit into from
Aug 15, 2023

Conversation

azaslavsky
Copy link
Contributor

Given two instances of a model that are being validated for equality, we want the foreign keys to be correct relatively (ie, they point to the same model in the respective models' JSON blobs), not absolutely (ie, they are literally the same integer). By creating maps that store the relations between pks and ordinals, we can easily check that the models point to match their respective ordinals regardless of the actual pk numbers.

Issue: getsentry/team-ospo#171

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Aug 10, 2023
@azaslavsky azaslavsky force-pushed the azaslavsky/backup/ignored_comparator branch from 25b28a6 to 7eeea6b Compare August 10, 2023 21:36
@azaslavsky azaslavsky force-pushed the azaslavsky/backup/foreign_key_comparator branch from 48a9d15 to ebb9fde Compare August 10, 2023 22:49
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Aug 10, 2023
@github-actions
Copy link
Contributor

🚨 Warning: This pull request contains Frontend and Backend changes!

It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently.

Have questions? Please ask in the #discuss-dev-infra channel.

@azaslavsky azaslavsky force-pushed the azaslavsky/backup/ignored_comparator branch from 7eeea6b to 225ff95 Compare August 10, 2023 23:12
@azaslavsky azaslavsky force-pushed the azaslavsky/backup/foreign_key_comparator branch from ebb9fde to fe35ff0 Compare August 10, 2023 23:12
@azaslavsky azaslavsky removed Scope: Frontend Automatically applied to PRs that change frontend components Scope: Backend Automatically applied to PRs that change backend components labels Aug 10, 2023
@azaslavsky azaslavsky force-pushed the azaslavsky/backup/ignored_comparator branch from 225ff95 to e421f0e Compare August 11, 2023 20:09
@azaslavsky azaslavsky force-pushed the azaslavsky/backup/foreign_key_comparator branch from fe35ff0 to 1e69cf7 Compare August 11, 2023 20:10
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Aug 11, 2023
@codecov
Copy link

codecov bot commented Aug 11, 2023

Codecov Report

Merging #54558 (258c205) into master (4adb453) will increase coverage by 0.00%.
The diff coverage is 93.10%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #54558   +/-   ##
=======================================
  Coverage   79.78%   79.78%           
=======================================
  Files        5000     5000           
  Lines      212243   212300   +57     
  Branches    36159    36173   +14     
=======================================
+ Hits       169331   169388   +57     
+ Misses      37707    37703    -4     
- Partials     5205     5209    +4     
Files Changed Coverage Δ
src/sentry/backup/validate.py 97.53% <80.00%> (-2.47%) ⬇️
src/sentry/backup/comparators.py 93.10% <94.11%> (+0.20%) ⬆️
src/sentry/backup/dependencies.py 90.90% <100.00%> (+1.11%) ⬆️
src/sentry/backup/findings.py 92.59% <100.00%> (+0.28%) ⬆️

... and 5 files with indirect coverage changes

Base automatically changed from azaslavsky/backup/ignored_comparator to master August 11, 2023 23:55
Given two instances of a model that are being validated for equality, we
want the foreign keys to be correct relatively (ie, they point to the
same model in the respective models' JSON blobs), not absolutely (ie,
they are literally the same integer). By creating maps that store the
relations between pks and ordinals, we can easily check that the models
point to match their respective ordinals regardless of the actual pk
numbers.

Issue: getsentry/team-ospo#171
@azaslavsky azaslavsky force-pushed the azaslavsky/backup/foreign_key_comparator branch from 1e69cf7 to 258c205 Compare August 14, 2023 18:33
@azaslavsky azaslavsky marked this pull request as ready for review August 14, 2023 19:10
@azaslavsky azaslavsky requested a review from a team August 14, 2023 19:10
Copy link
Member

@hubertdeng123 hubertdeng123 left a comment

Choose a reason for hiding this comment

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

This logic makes sense to me. Granted, I didn't know what the relationship between foreign keys and primary keys were before reviewing 😅

@azaslavsky azaslavsky merged commit 60f2390 into master Aug 15, 2023
@azaslavsky azaslavsky deleted the azaslavsky/backup/foreign_key_comparator branch August 15, 2023 14:33
@github-actions github-actions bot locked and limited conversation to collaborators Aug 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants