Rest proxy remove block from Schedulers #5376
Conversation
sima-zhu
requested review from
alzimmermsft,
hemanttanwar,
jianghaolu and
srnagar
as code owners
sdk/core/azure-core/src/main/java/com/azure/core/implementation/RestProxy.java
Outdated
Show resolved
Hide resolved
sdk/core/azure-core/src/main/java/com/azure/core/implementation/RestProxy.java
Outdated
Show resolved
Hide resolved
| response.getDecodedHeaders().block()); | ||
| return response.getDecodedHeaders() | ||
| .defaultIfEmpty(NullObject.INSTANCE) | ||
| .map(headers -> { |
There was a problem hiding this comment.
Should this be switched to a flatMap and follow a similar pattern as handleRestResponseReturnType which you changed in the PR as well.
There was a problem hiding this comment.
map works well in this case and make code less overhead.
I would prefer to keep using map here.
sdk/core/azure-core/src/main/java/com/azure/core/implementation/RestProxy.java
Outdated
Show resolved
Hide resolved
|
/azp run java -storage -tests |
|
No pipelines are associated with this pull request. |
|
/azp run java - storage - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| } catch (IllegalAccessException | InvocationTargetException | InstantiationException e) { | ||
| throw logger.logExceptionAsError(Exceptions.propagate(e)); | ||
| } | ||
| }; |
There was a problem hiding this comment.
nice work @sima-zhu :). I learned something today.
Just one question - Do we need an implementation of BiFunction to achieve this? or just a method is sufficient?, something like:
private static Response<?> createResponse(Constructor<? extends Response<?>> ctr, Object[] args) {
try {
return ctor.newInstance(args);
} catch (IllegalAccessException | InvocationTargetException | InstantiationException e) {
throw logger.logExceptionAsError(Exceptions.propagate(e));
}
}There was a problem hiding this comment.
Yes, I think it should be enough. Making functional as it would easy for any lambda function. Since it does not take function as parameter, I agree to convert it to method.
|
/azp run java - storage - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run java - eventhubs - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run java - keyvault - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
This is the fix which customer reported issue:
#5341
Here is the reactor issue for addressing the problem of invoking Mono/Flux.block calls inside NonBlockingThread.
reactor/reactor-core#1102