-
Notifications
You must be signed in to change notification settings - Fork 502
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
Fix confusing error msg. during 2Q creation #60
base: main
Are you sure you want to change the base?
Conversation
For cases where the combination of 2Q size and ghostRatio result in evict cache of size 0, the error message displayed to the user is rather confusing. This commit proposes to fix the code and give a suitable error message to the user. Addresses: Issue hashicorp#51
The error text you've added assumes the cause of errors but the error is coming from a different package. The better approach would be to go into the simplelru package and update the errors there, then keep this code the same, passing through the better error messages. |
@jefferai thanks for your comment but IMO my solution is correct. Reasons:
PS: Ideally the simplelru package could implement a custom error type for eg |
This is true now but may not be true in the future. This is why you should pass the error message through. |
@jefferai if I change the error message in New proposal: |
6b3f653
to
3c279a8
Compare
3c279a8
to
a8fae61
Compare
@jefferai what you think of the updated changes? |
For cases where the combination of 2Q size and ghostRatio result in
evict cache of size 0, the error message displayed to the user is
rather confusing.
This commit proposes to fix the code and give a suitable error message
to the user.
Addresses: Issue #51