-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Bug fix. Allow passing null http headers object to translateTableName #12764
Bug fix. Allow passing null http headers object to translateTableName #12764
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #12764 +/- ##
============================================
- Coverage 61.75% 61.53% -0.22%
+ Complexity 207 198 -9
============================================
Files 2436 2462 +26
Lines 133233 134627 +1394
Branches 20636 20839 +203
============================================
+ Hits 82274 82841 +567
- Misses 44911 45600 +689
- Partials 6048 6186 +138
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for addressing it! Does it make sense to put the @Nullable
annotation to the callers of these two methods, like handleRequest()
in BrokerRequestHandler
class?
Not sure if implementations of |
Not just the test code which will invoke the real logic, in fact a lot of companies will invoke the handleRequest() method with their own ways.
And I just tested the code before the PR mentioned above, previously the test did work with a null httpHeaders. But after the code change in that PR it starts to fail with a null httpHeaders. Could you add a unit test for this to make sure the original behavior won't break? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm merging it since the fix is very straight forward. We can discuss why null
values are passed in separately
Addresses issue #12745