fix(async-csv): Finishing touches on some async-csv bugs#18349
Conversation
|
|
||
| @instrumented_task(name="sentry.data_export.tasks.assemble_download", queue="data_export") | ||
| def assemble_download(data_export_id, limit=None, environment_id=None): | ||
| def assemble_download(data_export_id, limit=1000000, environment_id=None): |
There was a problem hiding this comment.
This is point 2. Enforces a hard limit of 1M rows
| const [data, _, response] = await api.requestPromise( | ||
| `/organizations/${slug}/data-export/`, | ||
| { | ||
| includeAllArgs: true, | ||
| method: 'POST', | ||
| data: { | ||
| query_type: queryType, | ||
| query_info: queryInfo, | ||
| }, | ||
| } | ||
| ); | ||
| const {id: dataExportId} = data; | ||
| addSuccessMessage( | ||
| t("Sit tight. We'll shoot you an email when your data is ready for download.") | ||
| response?.status === 201 | ||
| ? t("Sit tight. We'll shoot you an email when your data is ready for download.") | ||
| : t("It looks like we're already working on it. Sit tight, we'll email you.") |
There was a problem hiding this comment.
This is Point 1. Show a different toast message when the user refreshes the page and tries the same download. (There is a ticket to eventually save the state across refreshes, but it proves a little more difficult, so this is a temp fix)
| <Body> | ||
| <p>{errDetail}</p> | ||
| </Body> | ||
| )} |
There was a problem hiding this comment.
This is Point 3. If there isn't an error message, don't show this body section (as is the case in 404s)
| from enum import Enum | ||
|
|
||
| SNUBA_MAX_RESULTS = 1000 | ||
| SNUBA_MAX_RESULTS = 50000 |
There was a problem hiding this comment.
This is Point 4. Allow for more results per iteration (speeds up task by 50x). I believe the only limitation maybe memory issues by have a list that long with dicts in every spot. Not entirely sure on the memory limitations of workers in production so I think this is a good spot for now.
There was a problem hiding this comment.
Changed to 10k, since that's the actual max result limit. My bad!
leeandher
changed the title
2 participants
This PR is aimed at fixing up 4 smaller tasks.
When the user refreshes the page and clicks 'Export' again, we return a 200 (not a 201) because we didn't queue up another export. We just found their previous. Instead of swallowing this, the user now receives a different toast notification
As this will feature will likely go to GA soon, we're going to temporarily enforce a hard limit on file sizes at 1M rows.
The 404 response from the server doesn't have a
responseJSONfield. This change should still render an alright UI in case this happens (resolves JAVASCRIPT-222B)Increase the number of entries written to the CSV every iteration from 1000 to something higher. I just set it to
5000010000 as a safety net.