-
Notifications
You must be signed in to change notification settings - Fork 33
SO-6339: improve authorized context propagation #1346
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
Conversation
... SearchPageableCollectionResourceRequestBuilder Prefer using the context variant when there is a context available otherwise ensure that callers always supply the necessary request headers so that they are not failing with Missing authorization token exception when the initiating request was already successfully authenticated. Fixes https://snowowl.atlassian.net/browse/SO-6339
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 9.x #1346 +/- ##
============================================
- Coverage 63.67% 63.67% -0.01%
- Complexity 12523 12524 +1
============================================
Files 1612 1612
Lines 57120 57126 +6
Branches 5622 5622
============================================
+ Hits 36373 36376 +3
- Misses 18397 18401 +4
+ Partials 2350 2349 -1 ☔ 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.
RSLGTM!
} | ||
|
||
public final Stream<R> streamAsync(IEventBus bus, Map<String, String> headers, Function<B, AsyncRequest<R>> req) { | ||
return Streams.stream(new SearchResourceRequestIterator<B, R>(getSelf(), (builder) -> req.apply(builder).withHeaders(headers).execute(bus).getSync(3, TimeUnit.MINUTES))); |
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.
Can we document these timeouts?
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.
Somewhere yes, but that requires a bit more effort and there are a couple of these lying around in the code. Most of them are around 3 minutes. Probably we need to extract them into a static/dynamic config value. Will create a Jira for it.
When working with async streaming contexts the system was unable to propagate authorization information to the sub-request which was resulted in an error. This PR fixes that.
Jira: https://snowowl.atlassian.net/browse/SO-6339