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

Get validation database pipeline working behind API endpoint #203

Closed
Tracked by #153
azaslavsky opened this issue Sep 29, 2023 · 0 comments · Fixed by getsentry/sentry#59926
Closed
Tracked by #153

Get validation database pipeline working behind API endpoint #203

azaslavsky opened this issue Sep 29, 2023 · 0 comments · Fixed by getsentry/sentry#59926

Comments

@azaslavsky
Copy link

No description provided.

@azaslavsky azaslavsky mentioned this issue Sep 29, 2023
Closed
This was referenced Oct 2, 2023
Merged
Merged
@azaslavsky azaslavsky mentioned this issue Oct 2, 2023
Merged
@azaslavsky azaslavsky mentioned this issue Oct 2, 2023
Merged
azaslavsky added a commit to getsentry/sentry that referenced this issue Oct 3, 2023
@azaslavsky
fix(backup): Fix unique collision behavior
23dcf21 
There were bugs and inadequacies in the `unique` collision resolution
for imports - this change fixes them. Changes made here:

- ApiToken re-use is disallowed - this is dangerous! Instead, whenever
  public keys or secrets collide, the _importer_ gets a fresh new key.
- There were no collision tests for the following models with
  "collidable" unique sets: RelayUsage, Monitor, and UserPermission.
  These have now been added. A follow-on PR will add a check to
  `test_coverage` to ensure that all such models continue to be tested
  going forward.
- `UserEmail` resolution was incorrect for the `--merge_users` case.
  When merging users, we DO NOT want the email to be overwritten by the
  incoming user!
- OrgAuthToken, ProjectKey, and ProjectOption have all had their
  importers improved to make them a bit less confusing.

Besides this, some other code has been cleaned up a bit.

Issue: getsentry/team-ospo#203
azaslavsky added a commit to getsentry/sentry that referenced this issue Oct 3, 2023
@azaslavsky
test(backup): Add uniqueness and dynamic relocation coverage (#57301)
96fae5d 
Issue: getsentry/team-ospo#203
azaslavsky added a commit to getsentry/sentry that referenced this issue Oct 4, 2023
@azaslavsky
ref(backup): Swap actor refactor kludge (#57329)
3eaa971 
The previous relocation kludge for the actor refactor nulled out
`Actor.team_id`, making `Team` dependent on `Actor` which was dependent
on `User`. This was an odd arrangemnt - `Actor` was dependent on one of
the two models it sums over, but not the other.

This change flips the "cycle breaking" nulling operation, and does it on
`Team.actor_id` instead. That makes the dependency relationship much
more reflective of the concept we're trying to get at, with `Actor`
being dependent on both `Team` and `User`.

Issue: getsentry/team-ospo#203
azaslavsky added a commit to getsentry/sentry that referenced this issue Oct 5, 2023
@azaslavsky
fix(backup): Fix unique collision behavior (#57330)
932c110 
There were bugs and inadequacies in the `unique` collision resolution
for imports - this change fixes them. Changes made here:

- ApiToken re-use is disallowed - this is dangerous! Instead, whenever
public keys or secrets collide, the _importer_ gets a fresh new key.
- There were no collision tests for the following models with
"collidable" unique sets: RelayUsage, Monitor, and UserPermission. These
have now been added. A follow-on PR will add a check to `test_coverage`
to ensure that all such models continue to be tested going forward.
- `UserEmail` resolution was incorrect for the `--merge_users` case.
When merging users, we DO NOT want the email to be overwritten by the
incoming user!
- OrgAuthToken, ProjectKey, and ProjectOption have all had their
importers improved to make them a bit less confusing.

Besides this, some other code has been cleaned up a bit.

Issue: getsentry/team-ospo#203
azaslavsky added a commit to getsentry/sentry that referenced this issue Oct 6, 2023
@azaslavsky
ref(backup): Refine dangling model resolution
288e0fd 
Certain models that do not have unambiguous relationship to any
RelocationScope root model (ex: Organization, User, etc) are termed
"dangling", because filtering them down is done by simply moving up the
model dependency tree from the root to the leaves - we have to double
back and look at these "dangling" innner leaves instead. See
https://tinyurl.com/27z4x6tk for more info; the `SnubaQuery`,
`TimeSeriesSnapshot`, and `Email` models are all dangling in this
manner.

This PR changes the dangling resolution logic a bit: all `Excluded`
relocation scope models are no longer considered dangling. Additionally,
a few models that didn't quite get over the hump for "dangling"
resolution during status analysis, but are obviously so, are marked as
such.

Issue: getsentry/team-ospo#203
azaslavsky added a commit to getsentry/sentry that referenced this issue Oct 6, 2023
@azaslavsky
fix(backup): Fix query logic for dangling model exports
a79ef11 
There are a small number of models that have no unambiguous direct
connection to their relocation scope's root model - these are called
"dangling" models. The key factor that defines them, and makes them
difficult to handle, is that we cannot use our "query already exported
foreign keys" filtering methodology to select only the models relevant
to our export targets, because these models have no foreign keys that
connect them back to the root of that target. For example,
`TimeSeriesSnapshot` has no foreign keys at all, see:
https://tinyurl.com/27z4x6tk.

In cases like the one above, we ended up exporting ALL of the
`TimeSeriesSnapshot`s in the database - clearly a very bad outcome when
we only want to export those related to a specific org! A better
approach is to define custom filtering logic for these models, thereby
enabling them to use "adjacent" models in the model graph to select only
models that we care about for a given export. In the example above, we
query all `Incident`s filtered down by our previous exports to get a
sneak-peek at the set of `IncidentSnapshot`s (a set that is currently
empty due to going in reverse dependency order), then use that
information to work backwards to grab the `TimeSeriesSnapshot`s we need.

The upshot is that this commit introduces a generic method for
constructing filtered queries for a specific model, the overridable
`query_for_relocation_export`.

Issue: getsentry/team-ospo#203
This was referenced Oct 6, 2023
Merged
Merged
azaslavsky added a commit to getsentry/sentry that referenced this issue Oct 7, 2023
@azaslavsky
ref(backup): Refine dangling model resolution
11fcf1d 
Certain models that do not have unambiguous relationship to any
RelocationScope root model (ex: Organization, User, etc) are termed
"dangling", because filtering them down is done by simply moving up the
model dependency tree from the root to the leaves - we have to double
back and look at these "dangling" innner leaves instead. See
https://tinyurl.com/27z4x6tk for more info; the `SnubaQuery`,
`TimeSeriesSnapshot`, and `Email` models are all dangling in this
manner.

This PR changes the dangling resolution logic a bit: all `Excluded`
relocation scope models are no longer considered dangling. Additionally,
a few models that didn't quite get over the hump for "dangling"
resolution during status analysis, but are obviously so, are marked as
such.

Issue: getsentry/team-ospo#203
azaslavsky added a commit to getsentry/sentry that referenced this issue Oct 7, 2023
@azaslavsky
fix(backup): Fix query logic for dangling model exports
0ee6c61 
There are a small number of models that have no unambiguous direct
connection to their relocation scope's root model - these are called
"dangling" models. The key factor that defines them, and makes them
difficult to handle, is that we cannot use our "query already exported
foreign keys" filtering methodology to select only the models relevant
to our export targets, because these models have no foreign keys that
connect them back to the root of that target. For example,
`TimeSeriesSnapshot` has no foreign keys at all, see:
https://tinyurl.com/27z4x6tk.

In cases like the one above, we ended up exporting ALL of the
`TimeSeriesSnapshot`s in the database - clearly a very bad outcome when
we only want to export those related to a specific org! A better
approach is to define custom filtering logic for these models, thereby
enabling them to use "adjacent" models in the model graph to select only
models that we care about for a given export. In the example above, we
query all `Incident`s filtered down by our previous exports to get a
sneak-peek at the set of `IncidentSnapshot`s (a set that is currently
empty due to going in reverse dependency order), then use that
information to work backwards to grab the `TimeSeriesSnapshot`s we need.

The upshot is that this commit introduces a generic method for
constructing filtered queries for a specific model, the overridable
`query_for_relocation_export`.

Issue: getsentry/team-ospo#203
azaslavsky added a commit to getsentry/sentry that referenced this issue Oct 11, 2023
@azaslavsky
test(backup): Add coverage checks for unique collisions
6a1db02 
This ensures that we won't get `unique=True` collisions at import time.

Issue: getsentry/team-ospo#203
azaslavsky added a commit to getsentry/sentry that referenced this issue Nov 16, 2023
@azaslavsky
feat(backup): Support import chunking
cda57c5 
With this feature in place, we now atomically record which models we
imported in a given `import_by_model` call. This will be useful in the
short term for implementing the post-processing import step, and in the
long term to support rollbacks and partial import recovery.

Issue: getsentry/team-ospo#203
Issue: getsentry/team-ospo#213
azaslavsky added a commit to getsentry/sentry that referenced this issue Nov 16, 2023
@azaslavsky
feat(backup): Support import chunking (#59879)
5e05374 
With this feature in place, we now atomically record which models we
imported in a given `import_by_model` call. This will be useful in the
short term for implementing the post-processing import step, and in the
long term to support rollbacks and partial import recovery.

Issue: getsentry/team-ospo#203
Issue: getsentry/team-ospo#213
azaslavsky added a commit to getsentry/sentry that referenced this issue Nov 16, 2023
@azaslavsky
feat(backup): Relocation post-processing step
2e438da 
This task deals with assigning the owner of the relocation as the owner
of all of the organizations they have successfully imported.

Issue: getsentry/team-ospo#203
azaslavsky added a commit to getsentry/sentry that referenced this issue Nov 16, 2023
@azaslavsky
feat(backup): Relocation post-processing step (#59880)
8c9174c 
This task deals with assigning the owner of the relocation as the owner
of all of the organizations they have successfully imported.

Issue: getsentry/team-ospo#203
azaslavsky added a commit to getsentry/sentry that referenced this issue Nov 16, 2023
@azaslavsky
feat(backup): Send relocation started email
2145474 
Once we have decent confidence that a relocation is not obviously
invalid, we send the owner (and creator, if they are different people)
an email indicating that their relocation has started.

Issue: getsentry/team-ospo#203
azaslavsky added a commit to getsentry/sentry that referenced this issue Nov 16, 2023
@azaslavsky
feat(backup): Send relocation started email (#59922)
6079d39 
Once we have decent confidence that a relocation is not obviously
invalid, we send the owner (and creator, if they are different people)
an email indicating that their relocation has started.

![Screenshot 2023-11-15 at 15 39
35](https://github.com/getsentry/sentry/assets/3709945/580c7a82-984b-4f66-9b7d-3be761a39a90)

![Screenshot 2023-11-15 at 15 39
43](https://github.com/getsentry/sentry/assets/3709945/6d2884e9-0f40-4c24-9735-bad3c72c2823)

Issue: getsentry/team-ospo#203
azaslavsky added a commit to getsentry/sentry that referenced this issue Nov 16, 2023
@azaslavsky
feat(backup): Relocation failed email
cf5ee14 
In any case where a relocation definitively fails (not merely retrying a
failed task, but actually marking the relocation as a failure), we send
the owner and/or creator of the relocation an email to this effect.

Issue: getsentry/team-ospo#203
azaslavsky added a commit to getsentry/sentry that referenced this issue Nov 16, 2023
@azaslavsky
feat(backup): Relocation failed email (#59923)
5d71c86 
In any case where a relocation definitively fails (not merely retrying a
failed task, but actually marking the relocation as a failure), we send
the owner and/or creator of the relocation an email to this effect.

![Screenshot 2023-11-15 at 15 49
35](https://github.com/getsentry/sentry/assets/3709945/9b39dfdd-6384-4bce-928d-36e381d2e0fe)

![Screenshot 2023-11-15 at 15 49
44](https://github.com/getsentry/sentry/assets/3709945/04de61b1-7066-4124-97e7-6c408ce3ce1e)

Issue: getsentry/team-ospo#203
azaslavsky added a commit to getsentry/sentry that referenced this issue Nov 16, 2023
@azaslavsky
feat(backup): Relocation succeeded email
fdb0b05 
The last thing we do before marking a `Relocation` instance as a
`SUCCESS` is send an email to the owner and/or creator of that
relocation that it has completed successfully.

Issue: getsentry/team-ospo#203
azaslavsky added a commit to getsentry/sentry that referenced this issue Nov 16, 2023
@azaslavsky
feat(backup): Relocation succeeded email (#59924)
ac00924 
The last thing we do before marking a `Relocation` instance as a
`SUCCESS` is send an email to the owner and/or creator of that
relocation that it has completed successfully.

![Screenshot 2023-11-15 at 15 49
56](https://github.com/getsentry/sentry/assets/3709945/5f7f5468-5708-4965-aa66-e70554151b36)

![Screenshot 2023-11-15 at 15 50
03](https://github.com/getsentry/sentry/assets/3709945/7b68861c-1f1e-48d2-8ff0-f091ffd0e210)

Issue: getsentry/team-ospo#203
azaslavsky added a commit to getsentry/sentry that referenced this issue Nov 16, 2023
@azaslavsky
feat(backup): Notify users to claim accounts
d3da641 
This is the last relocation task! It actually sends out the "recover
your account" email to all imported users.

Issue: getsentry/team-ospo#203
azaslavsky added a commit to getsentry/sentry that referenced this issue Nov 16, 2023
@azaslavsky
feat(backup): Notify users to claim accounts (#59925)
f963d47 
This is the last relocation task! It actually sends out the "recover
your account" email to all imported users.

Issue: getsentry/team-ospo#203
azaslavsky added a commit to getsentry/sentry that referenced this issue Nov 16, 2023
@azaslavsky
ref(backup): Use burst task runner for better tests
64b56ee 
Instead of using the default task runner, which runs celery tasks at
their call sites and causes all sorts of shenanigans with nested
transactions, we switch to the "burst" task runner. This allows us to
add "max" retry test cases as well, and helped catch a couple of sneaky
bugs.

Closes getsentry/team-ospo#169
Closes getsentry/team-ospo#203
azaslavsky added a commit to getsentry/sentry that referenced this issue Nov 16, 2023
@azaslavsky
ref(backup): Use burst task runner for better tests (#59926)
9c867e1 
Instead of using the default task runner, which runs celery tasks at
their call sites and causes all sorts of shenanigans with nested
transactions, we switch to the "burst" task runner. This allows us to
add "max" retry test cases as well, and helped catch a couple of sneaky
bugs.

Closes getsentry/team-ospo#169
Closes getsentry/team-ospo#203
sentaur-athena pushed a commit to getsentry/sentry that referenced this issue Nov 16, 2023
@azaslavsky @sentaur-athena
feat(backup): Relocation failed email (#59923)
72d9c6e 
In any case where a relocation definitively fails (not merely retrying a
failed task, but actually marking the relocation as a failure), we send
the owner and/or creator of the relocation an email to this effect.

![Screenshot 2023-11-15 at 15 49
35](https://github.com/getsentry/sentry/assets/3709945/9b39dfdd-6384-4bce-928d-36e381d2e0fe)

![Screenshot 2023-11-15 at 15 49
44](https://github.com/getsentry/sentry/assets/3709945/04de61b1-7066-4124-97e7-6c408ce3ce1e)

Issue: getsentry/team-ospo#203
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

ref(backup): Use burst task runner for better tests getsentry/sentry
1 participant
@azaslavsky