refactor: Remove redundant parameters from SnapshotProducer validation methods #1853
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.
Which issue does this PR close?
What changes are included in this PR?
Summary
Refactor
SnapshotProducervalidation methods to use internal state instead of requiring redundant parameters.Problem
I've noticed that while the current SnapshotProducer API design already equips SnapshotProducer with all necessary state, the current invocations still redundantly pass parameters externally. I believe this could lead to some issues.
SnapshotProducer, leading to validating one set of files but committing anotherChanges
validate_added_data_files()andvalidate_duplicate_files()to operate onself.added_data_filesdirectlyFastAppendAction::commit()to call validation methods without passingadded_data_filesparameterMotivation
Previously,
added_data_fileswas passed as a parameter to validation methods even though it was already stored inSnapshotProducer:Benefits
Discussion
Since SnapshotProducer already holds all necessary state, can we further refine validation by performing it during the new function's execution to improve data consistency and encapsulation?
Are these changes tested?