-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Context passing between coroutines and Reactor Mono/Flux #1138
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
private const val SIGNALLED = -2L // already signalled subscriber onCompleted/onError | ||
|
||
@Suppress("CONFLICTING_JVM_DECLARATIONS", "RETURN_TYPE_MISMATCH_ON_INHERITANCE") | ||
private class PublisherCoroutine<in T>( |
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.
Copy-pasting the whole integration is definitely not an option: it is hard to maintain, hard to test and easy to mess up.
For example, we can make it public and mark with InternalCoroutinesApi
): Flux<T> = Flux.from(reactorPublish(newCoroutineContext(context), block = block)) | ||
|
||
@ExperimentalCoroutinesApi | ||
public fun <T> CoroutineScope.reactorPublish( |
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.
Why do we have to introduce public reactorPublish
API?
Integration should work with flux
builder.
|
||
class ReactorContextTest { | ||
@Test | ||
fun monoHookedContext() = runBlocking(Context.of(1, "1", 7, "7").asCoroutineContext()) { |
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.
code style: all our tests are started with test
prefix
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.
Great job!
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.
Good job! Please rebase it on current develop (it contains some changes in Flux.kt
and Publish.kt
), I will tweak documentation a bit and merge then
90cb996
to
ff06d83
Compare
ff06d83
to
2fa8ee9
Compare
67f7971
to
c2fd09f
Compare
No description provided.