Specify that OversizedSymbolTableHandler is required for limited buffers#1098
Specify that OversizedSymbolTableHandler is required for limited buffers#1098austnwil merged 2 commits intoamazon-ion:masterfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1098 +/- ##
============================================
+ Coverage 67.23% 67.98% +0.74%
- Complexity 5484 5636 +152
============================================
Files 159 162 +3
Lines 23025 23344 +319
Branches 4126 4179 +53
============================================
+ Hits 15481 15870 +389
+ Misses 6262 6181 -81
- Partials 1282 1293 +11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| try { | ||
| super.requireMaximumBufferSize(); | ||
| } catch(IllegalArgumentException e) { | ||
| throw new IllegalArgumentException("Must specify both an OversizedValueHandler and OversizedSymbolTableHandler when a custom maximum buffer size is specified."); | ||
| } |
There was a problem hiding this comment.
Instead of catch / re-throw, I'd suggest we just make requireMaximumBufferSize() private and have different implementations in the parent and subclass. It's simple enough that calling into super isn't saving us code anyway.
There was a problem hiding this comment.
I can make this change. Only difference would be that if you omit both, it will only warn you about OversizedValueHandler at first (because the check runs in both the parent and the subclass) and won't warn you about OversizedSymbolTableHandler until after you add the value handler.
There was a problem hiding this comment.
I'm fine with that. In the worst case a user will have to make two fixes, but each time the error message accurately points to a fix.
74de5e4 to
be4714f
Compare
tgregg
left a comment
There was a problem hiding this comment.
Use "Squash and Merge" for this instead of the default. Single commit to master.
Description of changes:
Errors from
IonBufferConfiguration.build()did not specify that OversizedSymbolTableHandlers are required in addition to OversizedValueHandlers if a maximum buffer size is set because the parent BufferConfiguration created the error message, which only knows about OversizedValueHandlers. This updates the error message to indicate that both are required.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.