Move DataExport CSV data into Active Storage #9409
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
Nitemaeric
force-pushed
the
dd/move-data-export-files-into-active-storage
branch
from
fab851c
to
0c1c121
Compare
Nitemaeric
requested review from
a team,
aje54 and
saliceti
and removed request for
a team and
aje54
tomas-stefano
requested changes
May 30, 2024
There was a problem hiding this comment.
I don't wanna to be picky but:
Is it possible to separate into multiples PRs? (DB migration/ERD, data migrations/code?)
Can we add documentation to the AKS cheatsheet about how to access the azure storage/buckets file, troubleshooting, etc?
Nitemaeric
force-pushed
the
dd/add-active-storage
branch
from
28f6c39
to
5ca08d3
Compare
Nitemaeric
force-pushed
the
dd/move-data-export-files-into-active-storage
branch
from
0c1c121
to
daa2870
Compare
dcyoung-dev
approved these changes
Jun 4, 2024
| @@ -48,7 +48,8 @@ def create | |||
| def download | |||
| data_export = DataExport.find(params[:id]) | |||
| data_export.update!(audit_comment: 'File downloaded') | |||
| send_data data_export.data, filename: data_export.filename, disposition: :attachment | |||
|
|
|||
| redirect_to rails_blob_path(data_export.file, disposition: 'attachment') | |||
There was a problem hiding this comment.
Should we check if data_export.file.attached? here?
There was a problem hiding this comment.
I took a slightly separate approach here. If the file doesn't exist, the .find call will fail.
Nitemaeric
force-pushed
the
dd/add-active-storage
branch
2 times, most recently
from
35c4782
to
f92e524
Compare
Nitemaeric
force-pushed
the
dd/move-data-export-files-into-active-storage
branch
from
daa2870
to
e0478de
Compare
Migrate previous `DataExport#data` values into `DataExport#file`. Start populating the CSV content into the `DataExport#file` field via the `DataExporter` class calls.
Nitemaeric
force-pushed
the
dd/move-data-export-files-into-active-storage
branch
from
e0478de
to
4b0a386
Compare
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.
Context
We want to reduce the size of the
data_exportstable rows. They are currently storing the CSV contents in the database itself in binary form.Changes proposed in this pull request
DataExportcsv data in thedataattribute into thefileattachment.data_exports#downloadcontroller action to download from Azure.Guidance to review
Locally
AZURE_STORAGE_ACCOUNT_NAME,AZURE_STORAGE_ACCESS_KEY, andAZURE_STORAGE_CONTAINER_NAMEenvironment variables accordingly.Documentation
Link to Trello card
https://trello.com/c/AS6YvTve/1342-bug-apply-db-backups-intermittently-failing
Things to check