Skip to content
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

Sending Multi<Byte> as files in multipart in REST Client Reactive #21569

Conversation

michalszynkiewicz
Copy link
Member

No description provided.

@michalszynkiewicz michalszynkiewicz force-pushed the rest-client-reactive-multi-as-file-multipart branch from 30a4eb1 to 44c8956 Compare November 29, 2021 08:58
@michalszynkiewicz michalszynkiewicz marked this pull request as ready for review November 29, 2021 08:59
@@ -68,7 +68,7 @@ public RestClientData getClientData() {

public RestClientData(List<RestClientInfo> clients, List<PossibleRestClientInfo> possibleClients) {
this.clients = clients;
this.possibleClients = possibleClients;
this.possibleClients = possibleClients; // TODO: present this info
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not related to this PR, I just didn't want to lose this TODO

@michalszynkiewicz michalszynkiewicz force-pushed the rest-client-reactive-multi-as-file-multipart branch 2 times, most recently from e3d9f4d to ffa1704 Compare November 29, 2021 09:00
@quarkus-bot
Copy link

quarkus-bot bot commented Nov 29, 2021

This workflow status is outdated as a new workflow run has been triggered.

🚫 This workflow run has been cancelled.

Failing Jobs - Building 44c8956

⚠️ Artifacts of the workflow run were not available thus the report misses some details.

Status Name Step Failures Logs Raw logs
Attach pull request number ⚠️ Check → Logs Raw logs
CI Sanity Check ⚠️ Check → Logs Raw logs

@quarkus-bot
Copy link

quarkus-bot bot commented Nov 29, 2021

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building ffa1704

Status Name Step Failures Logs Raw logs
JVM Tests - JDK 11 Build Failures Logs Raw logs
✔️ JVM Tests - JDK 17

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 11 #

- Failing: integration-tests/oidc-code-flow 

📦 integration-tests/oidc-code-flow

io.quarkus.it.keycloak.CodeFlowTest.testIdTokenInjectionWithoutRestoredPath line 403 - More details - Source on GitHub

com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException: 401 Unauthorized for http://localhost:8081/web-app/callback-after-redirect?state=50f9130f-f4c5-4e76-a835-d170f370036f&session_state=0fb6c27d-df72-4126-9544-d715e9d60cbf&code=81cf4a10-03ea-4077-bcdc-e0929fa8b43e.0fb6c27d-df72-4126-9544-d715e9d60cbf.169db77f-b8cf-4852-bc71-aef7c7a57017
	at com.gargoylesoftware.htmlunit.WebClient.throwFailingHttpStatusCodeExceptionIfNecessary(WebClient.java:701)
	at com.gargoylesoftware.htmlunit.WebClient.loadDownloadedResponses(WebClient.java:2452)

@michalszynkiewicz michalszynkiewicz force-pushed the rest-client-reactive-multi-as-file-multipart branch from ffa1704 to d567ab0 Compare November 29, 2021 12:35
@quarkus-bot
Copy link

quarkus-bot bot commented Nov 29, 2021

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building d567ab0

Status Name Step Failures Logs Raw logs
✔️ JVM Tests - JDK 11
JVM Tests - JDK 17 Build Failures Logs Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 17 #

- Failing: extensions/smallrye-reactive-messaging-kafka/deployment 
! Skipped: integration-tests/kafka-oauth-keycloak integration-tests/kafka-sasl-elytron integration-tests/kubernetes/quarkus-standard-way-kafka and 2 more

📦 extensions/smallrye-reactive-messaging-kafka/deployment

io.quarkus.smallrye.reactivemessaging.kafka.deployment.dev.KafkaDevServicesDevModeTestCase.sseStream - More details - Source on GitHub

java.lang.RuntimeException: 
java.lang.RuntimeException: java.lang.RuntimeException: java.lang.RuntimeException: java.lang.RuntimeException: java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.kafka.client.deployment.DevServicesKafkaProcessor#startKafkaDevService threw an exception: java.lang.RuntimeException: org.testcontainers.containers.ContainerLaunchException: Container startup failed

Copy link
Member

@cescoffier cescoffier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks very good. I would need to test it more.

There are some complicated classes that would deserve a bit of doc.

import org.jboss.logging.Logger;
import org.reactivestreams.Subscription;

public class MultiByteHttpData extends AbstractHttpData implements FileUpload {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should explain how this class is intended to work.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added some javadoc.

@@ -133,6 +160,10 @@ public Buffer content() {
return content;
}

public Multi<Byte> multiByteContent() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something we could improve (later) is to also support Multi<byte[]>, because it's what you receive most of the time (chunks, not bytes one by one).

@michalszynkiewicz michalszynkiewicz force-pushed the rest-client-reactive-multi-as-file-multipart branch from d567ab0 to b224f95 Compare December 2, 2021 08:13
@quarkus-bot
Copy link

quarkus-bot bot commented Dec 2, 2021

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building b224f95

Status Name Step Failures Logs Raw logs
Initial JDK 11 Build Build Failures Logs Raw logs

Failures

⚙️ Initial JDK 11 Build #

- Failing: independent-projects/resteasy-reactive/client/runtime 
! Skipped: devtools/bom-descriptor-json docs extensions/agroal/deployment and 214 more

📦 independent-projects/resteasy-reactive/client/runtime

Failed to execute goal net.revelc.code.formatter:formatter-maven-plugin:2.17.1:validate (default) on project resteasy-reactive-client: File '/home/runner/work/quarkus/quarkus/independent-projects/resteasy-reactive/client/runtime/src/main/java/org/jboss/resteasy/reactive/client/impl/multipart/MultiByteHttpData.java' has not been previously formatted. Please format file and commit before running validation!

@michalszynkiewicz michalszynkiewicz force-pushed the rest-client-reactive-multi-as-file-multipart branch from b224f95 to 395c5f0 Compare December 2, 2021 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants