Replace 2x LogUtils by QueryContext#101
Conversation
…tils`. Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>
Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>
Codecov Report
@@ Coverage Diff @@
## integ-query-context #101 +/- ##
======================================================
Coverage ? 97.76%
Complexity ? 2880
======================================================
Files ? 276
Lines ? 7078
Branches ? 447
======================================================
Hits ? 6920
Misses ? 157
Partials ? 1
Flags with carried forward coverage won't be shown. Click here to find out more. 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| */ | ||
| private static final String REQUEST_ID_KEY = "request_id"; | ||
|
|
||
| private static final String EMPTY_ID = "ID"; |
Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>
| public static String getRequestId() { | ||
| final String requestId = ThreadContext.get(REQUEST_ID_KEY); | ||
| return requestId; | ||
| return Optional.ofNullable(ThreadContext.get(REQUEST_ID_KEY)).orElseGet(() -> EMPTY_ID); |
There was a problem hiding this comment.
This should never happen, right?
Do we need all this code to protect ourselves from a situation that should never happen, or should we just allow the NullPointerException to get thrown...?
There was a problem hiding this comment.
Or better yet, let's call ThreadContext.addRequestId() if the REQUEST_ID_KEY doesn't exist instead of pass back a 'dummy' ID.
There was a problem hiding this comment.
Don't write functional-code for a test.
There was a problem hiding this comment.
It was written a time ago, I just renamed the file.
Please, see my fix in acf4ffa.
| public static String getRequestId() { | ||
| final String requestId = ThreadContext.get(REQUEST_ID_KEY); | ||
| return requestId; | ||
| return Optional.ofNullable(ThreadContext.get(REQUEST_ID_KEY)).orElseGet(() -> EMPTY_ID); |
There was a problem hiding this comment.
Or better yet, let's call ThreadContext.addRequestId() if the REQUEST_ID_KEY doesn't exist instead of pass back a 'dummy' ID.
Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>
Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>
legacy/src/test/java/org/opensearch/sql/legacy/unittest/utils/QueryContextTest.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>
| public static String getRequestId() { | ||
| final String requestId = ThreadContext.get(REQUEST_ID_KEY); | ||
| return requestId; | ||
| if (null == ThreadContext.get(REQUEST_ID_KEY)) { |
There was a problem hiding this comment.
if you chance addRequestId() to return the ID, you can return on that call.
| if (null == ThreadContext.get(REQUEST_ID_KEY)) { | ||
| addRequestId(); | ||
| } | ||
| return ThreadContext.get(REQUEST_ID_KEY); |
There was a problem hiding this comment.
you can calling ThreadContext.get(REQUEST_ID_KEY) twice. You can return if the value is !null.
Signed-off-by: Yury Fridlyand <yuryf@bitquilltech.com>
…_in_CI Include feature branch in workflow to trigger CI
Signed-off-by: Yury Fridlyand yuryf@bitquilltech.com
Description
Remove duplicated classes
Issues Resolved
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.