Fix app crash on big csv downloads #909
Conversation
![pullrequest[bot]](https://avatars.githubusercontent.com/in/3434?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ This pull request was sent to the PullRequest network.
Check the status or cancel PullRequest code review here.
There was a problem hiding this comment.
This is a great example of unbounded work causing issues, as per my notes on the previous PR that implemented this stuff. Even with this mitigations in place, I'd still recommend applying limits to restrict range size. Probably there is a maximum filesize supported by your load balancer, also? Other than this, the changes here are looking good, other than the notes left inline for your consideration.
Reviewed with ❤️ by PullRequest
packages/website/src/common/Chart/MultipleSensorsCharts/DownloadCSVButton.tsx
Outdated
Show resolved
Hide resolved
|
Build succeeded and deployed at https://aqualink-app-909.surge.sh |
| @@ -37,20 +30,16 @@ const Header = ({ site }: HeaderProps) => { | |||
| You can find example file formats here:{' '} | |||
| {exampleFiles.map((file, i) => ( | |||
| <span key={file}> | |||
| <Button | |||
| <a | |||
There was a problem hiding this comment.
Why did we need to make this change?
There was a problem hiding this comment.
these are some static files we point to, to download. It is just more straightforward to create a link with the file url, than to add custom download logic
There was a problem hiding this comment.
PullRequest reviewed the updates made to #909 since our last review was posted. This includes comments that have been posted by non-PullRequest reviewers. No further issues were found.
Reviewed by:
Chunks dates into 6 month intervals while creating the CSV. Appends each chunk to a temporally file, which will be sent as the response