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

"repo stat": Don't crash when Datastore.StorageMax is not defined #4246

Merged
merged 2 commits into from
Oct 3, 2017

Conversation

kevina
Copy link
Contributor

@kevina kevina commented Sep 18, 2017

Closes #4241

License: MIT
Signed-off-by: Kevin Atkinson [email protected]

@kevina
Copy link
Contributor Author

kevina commented Sep 18, 2017

@whyrusleeping note I use math.MaxUint64 as the default value in the stat structure to avoid an API change and then simply don't report it in the repo stat command if the value is math.MaxUint64

@whyrusleeping
Copy link
Member

@kevina I think '0' works as a default value too, might be a little easier to reason about

@kevina
Copy link
Contributor Author

kevina commented Sep 18, 2017

@whyrusleeping A very large value makes more logical sense. 0 could theoretically have a meaning as in no-storage. If the the of 0 is used for something bad things could happen. If a very large value is used, then it will effectively mean the same as no limit. That is why I think math.MaxUint64 makes more sence. If it was a floating point number I would use positive Infinity.

@kevina
Copy link
Contributor Author

kevina commented Sep 18, 2017

I can also make math.MaxUint64 a constant, say NoLimit, to make the code easier to read.

Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

LGTM. 2EiB is probably enough storage for a single node :).

@kevina kevina force-pushed the kevina/stat-no-max-storage branch from 6ab48f3 to 37f6a1b Compare September 18, 2017 23:26
@whyrusleeping whyrusleeping merged commit 4c39292 into master Oct 3, 2017
@whyrusleeping whyrusleeping deleted the kevina/stat-no-max-storage branch October 3, 2017 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants