-
Notifications
You must be signed in to change notification settings - Fork 19
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
Proper streaming ByteBody implementation #720
Conversation
With splitting and everything. After this, we can look at making some more binders between netty and servlet common, in particular the NettyBodyAnnotationBinder.
…byte-body # Conflicts: # servlet-engine/src/main/java/io/micronaut/servlet/engine/DefaultServletHttpRequest.java
servlet-engine/src/main/java/io/micronaut/servlet/engine/LazyDelegateInputStream.java
Show resolved
Hide resolved
servlet-engine/src/main/java/io/micronaut/servlet/engine/body/ExtendedInputStream.java
Outdated
Show resolved
Hide resolved
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.
Make sure you have high test coverage for all of these changes
*/ | ||
public DefaultServletHttpHandler(ApplicationContext applicationContext, ConversionService conversionService) { | ||
public DefaultServletHttpHandler(ApplicationContext applicationContext, ConversionService conversionService, @Named(TaskExecutors.BLOCKING) Executor ioExecutor) { |
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.
these should seems specific to Servlet, how will this work with GCP/AWS where we have the failing tests now?
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 searched for DefaultServletHttpRequest in micronaut-projects and this was the only usage, so I assume GCP/AWS still go through this handler
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.
AWS and GCP don't depend on servlet-engine
servlet-engine/src/main/java/io/micronaut/servlet/engine/DefaultServletHttpRequest.java
Show resolved
Hide resolved
servlet-engine/src/main/java/io/micronaut/servlet/engine/LazyDelegateInputStream.java
Show resolved
Hide resolved
servlet-engine/src/main/java/io/micronaut/servlet/engine/LazyDelegateInputStream.java
Outdated
Show resolved
Hide resolved
servlet-engine/src/main/java/io/micronaut/servlet/engine/body/ByteQueue.java
Outdated
Show resolved
Hide resolved
servlet-engine/src/main/java/io/micronaut/servlet/engine/body/ExtendedInputStream.java
Outdated
Show resolved
Hide resolved
servlet-engine/src/main/java/io/micronaut/servlet/engine/body/ExtendedInputStream.java
Outdated
Show resolved
Hide resolved
servlet-engine/src/main/java/io/micronaut/servlet/engine/body/InputStreamByteBody.java
Outdated
Show resolved
Hide resolved
servlet-engine/src/main/java/io/micronaut/servlet/engine/body/InputStreamByteBody.java
Outdated
Show resolved
Hide resolved
Re coverage: StreamPair is the complicated code here, and I tried to cover it pretty well. The rest of the code is covered by TCK tests |
sonar is broken unfortunately |
seems to be working now, please address the sonar issues |
Quality Gate failedFailed conditions |
With splitting and everything.
After this, we can look at making some more binders between netty and servlet common, in particular the NettyBodyAnnotationBinder.