You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "a-mazurok (via GitHub)" <gi...@apache.org> on 2023/10/25 15:03:13 UTC
[PR] Adding possibility to send binary messages to Azure Service Bus [camel]
a-mazurok opened a new pull request, #11838:
URL: https://github.com/apache/camel/pull/11838
# Description
Adding possibility to send binary messages to Azure Service Bus.
So far only string messages are supported in producer(all types are converted to string internally) while consumer supports both string and binary messages.
Binary messages are quire popular e.g. Protobuf messages.
# Target
- [yes] I checked that the commit is targeting the correct branch (note that Camel 3 uses `camel-3.x`, whereas Camel 4 uses the `main` branch)
# Tracking
- [yes] If this is a large change, bug fix, or code improvement, I checked there is a [JIRA issue](https://issues.apache.org/jira/browse/CAMEL) filed for the change (usually before you start working on it).
# Apache Camel coding standards and style
- [yes] I checked that each commit in the pull request has a meaningful subject line and body.
- [yes] I have run `mvn clean install -DskipTests` locally and I have committed all auto-generated changes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Adding possibility to send binary messages to Azure Service Bus [camel]
Posted by "davsclaus (via GitHub)" <gi...@apache.org>.
davsclaus commented on PR #11838:
URL: https://github.com/apache/camel/pull/11838#issuecomment-1780886563
> > I wonder if BinaryData can be created with InputStream as input source, eg if you do binary=true, and have a FileInputStream / File as message body, then IMHO it would be good that this can automatic convert this into BinaryData and use that. eg for big files then the file content is not needed to be loaded into memory, as byte[] otherwise would require.
>
> BinaryData can be created from both InputStream and file(as Path object) and I intentionally left BinaryData input body not converted to String or byte[](so binary parameter value is ignored in that case) so advanced users may convert their payloads to BinaryData in way they would prefer. For normal users that is unnecessary because ServiceBus message has size limitation of 256 KB only(except Premium tier) so they still can use byte arrays without risk to get OOM as almost nobody uses big messages in message brokers. Converting File or Input stream to byte array with help of Camel type converters should work really good and safe in such conditions.
Yes but we should let this component automatic do that. Your PR does not yet do that, does it?
1. At first use the body as if, if its one of these 3 that azure supports.
2. If binary=true, then create BinaryData if the body is InputStream or File. And if not then fallback and convert to byte[].
3. If binary=false, then use String or would byte[] be better?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Adding possibility to send binary messages to Azure Service Bus [camel]
Posted by "a-mazurok (via GitHub)" <gi...@apache.org>.
a-mazurok commented on code in PR #11838:
URL: https://github.com/apache/camel/pull/11838#discussion_r1372031655
##########
components/camel-azure/camel-azure-servicebus/src/main/java/org/apache/camel/component/azure/servicebus/ServiceBusProducer.java:
##########
@@ -147,12 +147,12 @@ private BiConsumer<Exchange, AsyncCallback> sendMessages() {
Mono<Void> sendMessageAsync;
- if (exchange.getMessage().getBody() instanceof Iterable) {
+ if (exchange.getMessage().getBody() instanceof Iterable<?>) {
sendMessageAsync
- = serviceBusSenderOperations.sendMessages(convertBodyToList((Iterable<Object>) inputBody),
+ = serviceBusSenderOperations.sendMessages(convertBodyToList((Iterable<?>) inputBody),
configurationOptionsProxy.getServiceBusTransactionContext(exchange), applicationProperties);
} else {
- sendMessageAsync = serviceBusSenderOperations.sendMessages(exchange.getMessage().getBody(String.class),
+ sendMessageAsync = serviceBusSenderOperations.sendMessages(inputBody,
Review Comment:
As I can see both consumers and producers will support only String, byte[] and BinaryData and only those types(besides special AmqpMessageBody and ServiceBusReceivedMessage types) are supported by underlying Azure library.
I think that client developer can always convert any other Java type to String if necessary in proper way(including Camel type converters) than doing it internally and implicitly with toString() method or implicit Camel type converters(as it works so far).
Binary data like Protobuf messages(our case) can't be properly converted to String type and I believe that many other organizations may have met the same limitation already.
I understand that potentially that change can break some potentially existing clients who were relying on such implicit conversion of different Java types to String so such change shouldn't be released as minor version upgrade 4.1.X but I hope it can be included in 4.2.0 release together with documentation update.
Maybe we can add the same change in new Camel 3.X.0 release as well(if any planned).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Adding possibility to send binary messages to Azure Service Bus [camel]
Posted by "a-mazurok (via GitHub)" <gi...@apache.org>.
a-mazurok commented on code in PR #11838:
URL: https://github.com/apache/camel/pull/11838#discussion_r1372043405
##########
components/camel-azure/camel-azure-servicebus/src/main/java/org/apache/camel/component/azure/servicebus/ServiceBusProducer.java:
##########
@@ -147,12 +147,12 @@ private BiConsumer<Exchange, AsyncCallback> sendMessages() {
Mono<Void> sendMessageAsync;
- if (exchange.getMessage().getBody() instanceof Iterable) {
+ if (exchange.getMessage().getBody() instanceof Iterable<?>) {
sendMessageAsync
- = serviceBusSenderOperations.sendMessages(convertBodyToList((Iterable<Object>) inputBody),
+ = serviceBusSenderOperations.sendMessages(convertBodyToList((Iterable<?>) inputBody),
configurationOptionsProxy.getServiceBusTransactionContext(exchange), applicationProperties);
} else {
- sendMessageAsync = serviceBusSenderOperations.sendMessages(exchange.getMessage().getBody(String.class),
+ sendMessageAsync = serviceBusSenderOperations.sendMessages(inputBody,
Review Comment:
In case of other Java types `ServiceBusUtils` will throw `IllegalArgumentException("Make sure your message data is in String, byte[] or BinaryData")`(not changed that).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Adding possibility to send binary messages to Azure Service Bus [camel]
Posted by "a-mazurok (via GitHub)" <gi...@apache.org>.
a-mazurok commented on PR #11838:
URL: https://github.com/apache/camel/pull/11838#issuecomment-1783357342
> > > I wonder if BinaryData can be created with InputStream as input source, eg if you do binary=true, and have a FileInputStream / File as message body, then IMHO it would be good that this can automatic convert this into BinaryData and use that. eg for big files then the file content is not needed to be loaded into memory, as byte[] otherwise would require.
> >
> >
> > BinaryData can be created from both InputStream and file(as Path object) and I intentionally left BinaryData input body not converted to String or byte[](so binary parameter value is ignored in that case) so advanced users may convert their payloads to BinaryData in way they would prefer. For normal users that is unnecessary because ServiceBus message has size limitation of 256 KB only(except Premium tier) so they still can use byte arrays without risk to get OOM as almost nobody uses big messages in message brokers. Converting File or Input stream to byte array with help of Camel type converters should work really good and safe in such conditions.
>
> Yes but we should let this component automatic do that. Your PR does not yet do that, does it?
>
> 1. At first use the body as if, if its one of these 3 that azure supports.
> 2. If binary=true, then create BinaryData if the body is InputStream or File. And if not then fallback and convert to byte[].
> 3. If binary=false, then use String or would byte[] be better?
Applied changes and added uncommitted earlier generated files.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Adding possibility to send binary messages to Azure Service Bus [camel]
Posted by "davsclaus (via GitHub)" <gi...@apache.org>.
davsclaus commented on PR #11838:
URL: https://github.com/apache/camel/pull/11838#issuecomment-1779945323
I wonder if BinaryData can be created with InputStream as input source, eg if you do binary=true, and have a FileInputStream / File as message body, then IMHO it would be good that this can automatic convert this into BinaryData and use that. eg for big files then the file content is not needed to be loaded into memory, as byte[] otherwise would require.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Adding possibility to send binary messages to Azure Service Bus [camel]
Posted by "a-mazurok (via GitHub)" <gi...@apache.org>.
a-mazurok commented on code in PR #11838:
URL: https://github.com/apache/camel/pull/11838#discussion_r1372031655
##########
components/camel-azure/camel-azure-servicebus/src/main/java/org/apache/camel/component/azure/servicebus/ServiceBusProducer.java:
##########
@@ -147,12 +147,12 @@ private BiConsumer<Exchange, AsyncCallback> sendMessages() {
Mono<Void> sendMessageAsync;
- if (exchange.getMessage().getBody() instanceof Iterable) {
+ if (exchange.getMessage().getBody() instanceof Iterable<?>) {
sendMessageAsync
- = serviceBusSenderOperations.sendMessages(convertBodyToList((Iterable<Object>) inputBody),
+ = serviceBusSenderOperations.sendMessages(convertBodyToList((Iterable<?>) inputBody),
configurationOptionsProxy.getServiceBusTransactionContext(exchange), applicationProperties);
} else {
- sendMessageAsync = serviceBusSenderOperations.sendMessages(exchange.getMessage().getBody(String.class),
+ sendMessageAsync = serviceBusSenderOperations.sendMessages(inputBody,
Review Comment:
As I can see both consumers and producers will support only String, byte[] and BinaryData and only those types(besides special AmqpMessageBody and ServiceBusReceivedMessage types) are supported by underlying Azure library.
I think that client developer can always convert any other Java type to String if necessary in proper way(including Camel type converters) than doing it internally and implicitly with toString() method or implicit Camel type converters.
Binary data like Protobuf messages(our case) can't be properly converted to String type and I believe that many other organizations may have met the same limitation already.
I understand that potentially that change can break some potentially existing clients who were relying on such implicit conversion of different Java types to String so such change shouldn't be released as minor version upgrade 4.1.X but I hope it can be included in 4.2.0 release together with documentation update.
Maybe we can add the same change in new Camel 3.X.0 release as well(if any planned).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Adding possibility to send binary messages to Azure Service Bus [camel]
Posted by "a-mazurok (via GitHub)" <gi...@apache.org>.
a-mazurok commented on code in PR #11838:
URL: https://github.com/apache/camel/pull/11838#discussion_r1372068319
##########
components/camel-azure/camel-azure-servicebus/src/main/java/org/apache/camel/component/azure/servicebus/ServiceBusProducer.java:
##########
@@ -147,12 +147,12 @@ private BiConsumer<Exchange, AsyncCallback> sendMessages() {
Mono<Void> sendMessageAsync;
- if (exchange.getMessage().getBody() instanceof Iterable) {
+ if (exchange.getMessage().getBody() instanceof Iterable<?>) {
sendMessageAsync
- = serviceBusSenderOperations.sendMessages(convertBodyToList((Iterable<Object>) inputBody),
+ = serviceBusSenderOperations.sendMessages(convertBodyToList((Iterable<?>) inputBody),
configurationOptionsProxy.getServiceBusTransactionContext(exchange), applicationProperties);
} else {
- sendMessageAsync = serviceBusSenderOperations.sendMessages(exchange.getMessage().getBody(String.class),
+ sendMessageAsync = serviceBusSenderOperations.sendMessages(inputBody,
Review Comment:
Yes, BinaryData is Azure special class that is like wrapper that can hold both Strings and bytes inside.
If I correctly understand your thoughts direction then probably some new endpoint configuration property should be added e.g. `binary` with default value false(backward compatible) that will control if Camel should try to convert any Java class to String or byte[] unless it is already BinaryData type that doesn't need any conversion.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Adding possibility to send binary messages to Azure Service Bus [camel]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #11838:
URL: https://github.com/apache/camel/pull/11838#issuecomment-1779481843
:star2: Thank you for your contribution to the Apache Camel project! :star2:
:robot: CI automation will test this PR automatically.
:camel: Apache Camel Committers, please review the following items:
* First-time contributors **require MANUAL approval** for the GitHub Actions to run
* You can use the command `/component-test (camel-)component-name1 (camel-)component-name2..` to request a test from the test bot.
* You can label PRs using `build-all`, `build-dependents`, `skip-tests` and `test-dependents` to fine-tune the checks executed by this PR.
* Build and test logs are available in the Summary page. **Only** [Apache Camel committers](https://camel.apache.org/community/team/#committers) have access to the summary.
* :warning: Be careful when sharing logs. Review their contents before sharing them publicly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Adding possibility to send binary messages to Azure Service Bus [camel]
Posted by "a-mazurok (via GitHub)" <gi...@apache.org>.
a-mazurok commented on PR #11838:
URL: https://github.com/apache/camel/pull/11838#issuecomment-1780853563
> I wonder if BinaryData can be created with InputStream as input source, eg if you do binary=true, and have a FileInputStream / File as message body, then IMHO it would be good that this can automatic convert this into BinaryData and use that. eg for big files then the file content is not needed to be loaded into memory, as byte[] otherwise would require.
BinaryData can be created from both InputStream and file(as Path object) and I intentionally left BinaryData input body not converted to String or byte[] so advanced users may convert their payloads to BinaryData in way they would prefer.
For normal users that is unnecessary because ServiceBus message has size limitation of 256 KB only(except Premium tier) so they still can use byte arrays without risk to get OOM.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Adding possibility to send binary messages to Azure Service Bus [camel]
Posted by "davsclaus (via GitHub)" <gi...@apache.org>.
davsclaus commented on code in PR #11838:
URL: https://github.com/apache/camel/pull/11838#discussion_r1372060730
##########
components/camel-azure/camel-azure-servicebus/src/main/java/org/apache/camel/component/azure/servicebus/ServiceBusProducer.java:
##########
@@ -147,12 +147,12 @@ private BiConsumer<Exchange, AsyncCallback> sendMessages() {
Mono<Void> sendMessageAsync;
- if (exchange.getMessage().getBody() instanceof Iterable) {
+ if (exchange.getMessage().getBody() instanceof Iterable<?>) {
sendMessageAsync
- = serviceBusSenderOperations.sendMessages(convertBodyToList((Iterable<Object>) inputBody),
+ = serviceBusSenderOperations.sendMessages(convertBodyToList((Iterable<?>) inputBody),
configurationOptionsProxy.getServiceBusTransactionContext(exchange), applicationProperties);
} else {
- sendMessageAsync = serviceBusSenderOperations.sendMessages(exchange.getMessage().getBody(String.class),
+ sendMessageAsync = serviceBusSenderOperations.sendMessages(inputBody,
Review Comment:
Ok thanks, but we need to leverage Camels type converter systems, so eg if your body is a `java.io.File` or `InputStream` then they can be sent as-is also.
And is BinaryData some kind of azure class ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Adding possibility to send binary messages to Azure Service Bus [camel]
Posted by "davsclaus (via GitHub)" <gi...@apache.org>.
davsclaus commented on code in PR #11838:
URL: https://github.com/apache/camel/pull/11838#discussion_r1372124086
##########
components/camel-azure/camel-azure-servicebus/src/main/java/org/apache/camel/component/azure/servicebus/ServiceBusProducer.java:
##########
@@ -147,12 +147,12 @@ private BiConsumer<Exchange, AsyncCallback> sendMessages() {
Mono<Void> sendMessageAsync;
- if (exchange.getMessage().getBody() instanceof Iterable) {
+ if (exchange.getMessage().getBody() instanceof Iterable<?>) {
sendMessageAsync
- = serviceBusSenderOperations.sendMessages(convertBodyToList((Iterable<Object>) inputBody),
+ = serviceBusSenderOperations.sendMessages(convertBodyToList((Iterable<?>) inputBody),
configurationOptionsProxy.getServiceBusTransactionContext(exchange), applicationProperties);
} else {
- sendMessageAsync = serviceBusSenderOperations.sendMessages(exchange.getMessage().getBody(String.class),
+ sendMessageAsync = serviceBusSenderOperations.sendMessages(inputBody,
Review Comment:
Yeah that sounds like a good idea.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Adding possibility to send binary messages to Azure Service Bus [camel]
Posted by "a-mazurok (via GitHub)" <gi...@apache.org>.
a-mazurok commented on code in PR #11838:
URL: https://github.com/apache/camel/pull/11838#discussion_r1372249368
##########
components/camel-azure/camel-azure-servicebus/src/main/java/org/apache/camel/component/azure/servicebus/ServiceBusProducer.java:
##########
@@ -147,12 +147,12 @@ private BiConsumer<Exchange, AsyncCallback> sendMessages() {
Mono<Void> sendMessageAsync;
- if (exchange.getMessage().getBody() instanceof Iterable) {
+ if (exchange.getMessage().getBody() instanceof Iterable<?>) {
sendMessageAsync
- = serviceBusSenderOperations.sendMessages(convertBodyToList((Iterable<Object>) inputBody),
+ = serviceBusSenderOperations.sendMessages(convertBodyToList((Iterable<?>) inputBody),
configurationOptionsProxy.getServiceBusTransactionContext(exchange), applicationProperties);
} else {
- sendMessageAsync = serviceBusSenderOperations.sendMessages(exchange.getMessage().getBody(String.class),
+ sendMessageAsync = serviceBusSenderOperations.sendMessages(inputBody,
Review Comment:
Added new property "binary".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Adding possibility to send binary messages to Azure Service Bus [camel]
Posted by "davsclaus (via GitHub)" <gi...@apache.org>.
davsclaus commented on PR #11838:
URL: https://github.com/apache/camel/pull/11838#issuecomment-1779946244
eg BinaryData is azure specifc and end users dont know about this, and its a hazzle for them to write code that creates a BinaryData object and populate it, to use in Camel
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Adding possibility to send binary messages to Azure Service Bus [camel]
Posted by "davsclaus (via GitHub)" <gi...@apache.org>.
davsclaus commented on code in PR #11838:
URL: https://github.com/apache/camel/pull/11838#discussion_r1371949444
##########
components/camel-azure/camel-azure-servicebus/src/main/java/org/apache/camel/component/azure/servicebus/ServiceBusProducer.java:
##########
@@ -147,12 +147,12 @@ private BiConsumer<Exchange, AsyncCallback> sendMessages() {
Mono<Void> sendMessageAsync;
- if (exchange.getMessage().getBody() instanceof Iterable) {
+ if (exchange.getMessage().getBody() instanceof Iterable<?>) {
sendMessageAsync
- = serviceBusSenderOperations.sendMessages(convertBodyToList((Iterable<Object>) inputBody),
+ = serviceBusSenderOperations.sendMessages(convertBodyToList((Iterable<?>) inputBody),
configurationOptionsProxy.getServiceBusTransactionContext(exchange), applicationProperties);
} else {
- sendMessageAsync = serviceBusSenderOperations.sendMessages(exchange.getMessage().getBody(String.class),
+ sendMessageAsync = serviceBusSenderOperations.sendMessages(inputBody,
Review Comment:
What if the input body is a Java class or somethig else. Does ASB know how to handle all kind of java objects ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Adding possibility to send binary messages to Azure Service Bus [camel]
Posted by "davsclaus (via GitHub)" <gi...@apache.org>.
davsclaus merged PR #11838:
URL: https://github.com/apache/camel/pull/11838
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org