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

renamed fullArchive to cleanOldEpochsData #2084

Merged
merged 6 commits into from
Jul 10, 2020

Conversation

bogdan-rosianu
Copy link
Contributor

renamed boolean parameter FullArchive to CleanOldEpochsData

@bogdan-rosianu bogdan-rosianu added the type:feature New feature or request label Jul 9, 2020
@bogdan-rosianu bogdan-rosianu self-assigned this Jul 9, 2020
sasurobert
sasurobert previously approved these changes Jul 9, 2020
@@ -40,7 +40,7 @@ func NewStorageServiceFactory(
if config == nil {
return nil, storage.ErrNilConfig
}
if config.StoragePruning.NumEpochsToKeep < minimumNumberOfEpochsToKeep && !config.StoragePruning.FullArchive {
if config.StoragePruning.NumEpochsToKeep < minimumNumberOfEpochsToKeep && !config.StoragePruning.CleanOldEpochsData {
Copy link
Contributor

Choose a reason for hiding this comment

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

why is cleanOldEpochsData relevant here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the node doesn't remove old epochs, then there is no use for the NumEpochsToKeep so the check shouldn't be blocking. I can remove the condition if needed. Also, thanks for finding this because I have to change the boolean condition

Copy link
Contributor

Choose a reason for hiding this comment

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

ok then you can keep the check, but remove negation

AdoAdoAdo
AdoAdoAdo previously approved these changes Jul 10, 2020
Copy link
Contributor

@LucianMincu LucianMincu left a comment

Choose a reason for hiding this comment

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

System tests passed.

@LucianMincu LucianMincu merged commit 60d8cf1 into development Jul 10, 2020
@LucianMincu LucianMincu deleted the renaming-pruning-storer branch July 10, 2020 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants