-
-
Notifications
You must be signed in to change notification settings - Fork 57
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
feat: new page in snuba admin to do deletes #6157
Merged
Merged
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
kylemumma
force-pushed
the
krm/admin-delete
branch
from
July 30, 2024 15:21
6cedd7e
to
bc42a4f
Compare
kylemumma
commented
Jul 30, 2024
snuba/web/views.py
Outdated
payload = delete_from_storage(storage, columns) | ||
results = delete_from_storage(storage, columns) | ||
except ValueError as error: | ||
return make_response(jsonify({"error": str(error)}), 400) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this catches the new columns
validation added to delete_from_storage
Not too important, but it'd be nice to have a screenshot of what your UI looks like, kinda like this: #5883 (comment) |
@xurui-c I added a screenshot to the description |
kylemumma
force-pushed
the
krm/admin-delete
branch
from
July 31, 2024 16:05
ff5714e
to
45c69dd
Compare
MeredithAnya
approved these changes
Jul 31, 2024
PR reverted: 02b598a |
getsentry-bot
added a commit
that referenced
this pull request
Jul 31, 2024
This reverts commit 1aab7d6. Co-authored-by: kylemumma <24424170+kylemumma@users.noreply.github.com>
MeredithAnya
pushed a commit
that referenced
this pull request
Jul 31, 2024
this is a redo of #6157 This PR adds a page to snuba admin that we can use to test deletes in prod. ## major changes 1. create a front-end react interface for deletes * `static/delete_tool/index.tsx` is the new react UI, it calls `api_client.tsx:runLightweightDelete` * `api_client.py:runLightweightDelete` sends the input via http request to `/delete` endpoint. (note: i think it would have been equally valid to just send the http request from index.tsx rather than having this separate function, but I did it like this bc everyone else in the codebase did) * `data.tsx` adds the new page to the Nav bar 2. create a new `/delete` endpoint in snuba admin `admin/views.py:delete` As said in bullet 1, the react UI sends the input to `/delete`. This is a new endpoint we created, it mostly just calls `delete_from_storage` (what Meredith implemented that actually does the delete) 3. set permissions for `/delete` endpoint and UI `tool_policies.py` It just goes into AllTools by default which is the most restrictive role, only snuba team and people with access to everything can use it. I think this should be good. 4. small `delete_query.py` changes * I slightly changed the input/output types of `delete_query.py:delete_from_storage` in order to make the interface more strict/clear. * added more input verification for the `columns` input. ## considerations This tool is pretty much just a layer on top of `delete_from_storage` meaning it can do anything that `delete_from_storage` can do. Hence it will be able to delete in any environment, and any dataset that has deletes enabled. Is this fine or should we add additional strictness to this interface, so that it can only be used for search_issues in s4s no matter what. <img width="1212" alt="Screenshot 2024-07-30 at 1 10 29 PM" src="https://github.com/user-attachments/assets/d3ad9005-69b1-4995-8934-f85494fdb3b5">
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.
This PR adds a page to snuba admin that we can use to test deletes in prod.
major changes
static/delete_tool/index.tsx
is the new react UI, it callsapi_client.tsx:runLightweightDelete
api_client.py:runLightweightDelete
sends the input via http request to/delete
endpoint. (note: i think it would have been equally valid to just send the http request from index.tsx rather than having this separate function, but I did it like this bc everyone else in the codebase did)data.tsx
adds the new page to the Nav bar/delete
endpoint in snuba adminadmin/views.py:delete
As said in bullet 1, the react UI sends the input to
/delete
. This is a new endpoint we created, it mostly just callsdelete_from_storage
(what Meredith implemented that actually does the delete)/delete
endpoint and UItool_policies.py
It just goes into AllTools by default which is the most restrictive role, only snuba team and people with access to everything can use it. I think this should be good.
delete_query.py
changesdelete_query.py:delete_from_storage
in order to make the interface more strict/clear.columns
input.considerations
This tool is pretty much just a layer on top of
delete_from_storage
meaning it can do anything thatdelete_from_storage
can do. Hence it will be able to delete in any environment, and any dataset that has deletes enabled. Is this fine or should we add additional strictness to this interface, so that it can only be used for search_issues in s4s no matter what.