-
Notifications
You must be signed in to change notification settings - Fork 57
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
RUMM-2235: Rework file persistence layer #947
RUMM-2235: Rework file persistence layer #947
Conversation
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.
LGTM
// is requested with granted orchestrator (due to consent change). Not an issue, because | ||
// batch file should be migrated to the same folder, but leaving this debug point | ||
// just in case. | ||
internalLogger.debugWithTelemetry( |
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.
Nice 👍
.join( | ||
separator = decoration.separatorBytes, | ||
prefix = decoration.prefixBytes, | ||
suffix = decoration.suffixBytes | ||
) | ||
uploader.upload(batch) | ||
handler.delete(it) | ||
fileMover.delete(it) | ||
val metaFile = fileOrchestrator.getMetadataFile(it) |
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.
where is this data uploaded if you delete it here ?
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.
The data is uploaded few lines above (uploader.upload(batch)
), basically the logic is the same, I just added metadata deletion.
But I see your concern: metadata is requested for the file which is deleted and while it is ok to have (File
object stays there still and we just use its path to get associated metadata file), I swapped the lines - now metadata file is requested before batch file deletion, which makes the operation more clear.
fea0141
to
9b09887
Compare
What does this PR do?
This change makes some re-work of the file persistence layer to answer the new requirements:
To support these requirements the following was done in this PR:
FileMover
(not the best name, I know, butFileOrchestrator
was already taken).FileOrchestrator
- this gives an advantage for the proper file cleanup once batch is dropped, expired or simply disk space should be freed.Review checklist (to be filled by reviewers)