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.

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

Using streams on schema export service #319

Merged
merged 25 commits into from
Aug 4, 2023
Merged

Conversation

Beckyrose200
Copy link
Contributor

https://eaflood.atlassian.net/browse/WATER-4019

This PR's primary focus is to fix the schema export service. The service is currently not working due to the fact it reaches its memory capacity when it exports the returns table. This is due to the fact the full table is brought into memory. This fix introduced streams into the service to allow the table to be written immediately to a file and not be held in full in memory.

https://eaflood.atlassian.net/browse/WATER-4019

This PR's primary focus is to fix the schema export service. The service is currently not working due to the fact it reaches its memory capacity when it exports the returns table. This is due to the fact the full table is brought into memory. This fix introduced streams into the service to allow the table to be written immediately to a file and not be held in full in memory.
@Beckyrose200 Beckyrose200 added the bug Something isn't working label Jul 25, 2023
@Beckyrose200 Beckyrose200 self-assigned this Jul 25, 2023
@Beckyrose200 Beckyrose200 marked this pull request as ready for review August 2, 2023 16:29
Copy link
Contributor

@StuAA78 StuAA78 left a comment

Choose a reason for hiding this comment

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

This is looking great, just a few changes to consider.

app/services/data/export/fetch-table.service.js Outdated Show resolved Hide resolved
app/services/data/export/write-stream-to-file.service.js Outdated Show resolved Hide resolved
app/services/data/export/write-stream-to-file.service.js Outdated Show resolved Hide resolved
app/services/data/export/write-stream-to-file.service.js Outdated Show resolved Hide resolved
app/services/data/export/write-stream-to-file.service.js Outdated Show resolved Hide resolved
app/services/data/export/write-stream-to-file.service.js Outdated Show resolved Hide resolved
app/services/data/export/convert-to-csv.service.js Outdated Show resolved Hide resolved
@Beckyrose200 Beckyrose200 requested a review from StuAA78 August 4, 2023 09:17
Copy link
Contributor

@StuAA78 StuAA78 left a comment

Choose a reason for hiding this comment

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

@Beckyrose200 Beckyrose200 merged commit 47b73d1 into main Aug 4, 2023
@Beckyrose200 Beckyrose200 deleted the new-schema-export branch August 4, 2023 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants