-
Notifications
You must be signed in to change notification settings - Fork 915
Account import & export #2356
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
base: main
Are you sure you want to change the base?
Account import & export #2356
Conversation
ffabdbd to
9e37050
Compare
9e37050 to
fec5819
Compare
| def import_batch(files) | ||
| batch_data = files.map do |file| | ||
| data = load(file) | ||
| data["body"] = convert_gids_to_sgids(data["body"]) |
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.
If we did try to collapse the record set classes a bit, we could handle this in a sort of before_insert step so that we don't need to supply a full implementation of import_batch to do it.
I also wonder if the export half of this is necessary (converting sgids to gids) or could we implicitly trust the signing on the way in while converting to new sgids? Effectively re-signing from an unknown key to the valid one. Since the transfer is effectively trusting that aspect of the input data anyway, it could be one less step. (Not sure if it makes much difference in practice or not though.)
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.
I opted to go with a generic RecordSet and specific record sets. That kept the interface simple.
For the conversion. Yes, I could skip the SGID > GID > SGID dance, though this feels nicer.
ZipKit can read and write files directly from S3 which makes both imports and exports way more efficient in terms of disk usage.
During import, card return nil since it doesn't exist yet
Validate is an ActiveModel method, overriding it could have unintended side-effects so I opted to rename it
The old implementation loaded files into memory to provide an IO interface. This has the obvious downside of loading any file included in the import, e.g. a 10GB video file, into memory. ZipKit has no native IO object for reading but it provides all the necessary methods to implement one.
No description provided.