You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2022/04/22 13:42:11 UTC

[GitHub] [activemq-artemis] MarcoR83 opened a new pull request, #4040: Allow binary deserialization of message properties AMQ_MSG_MESSAGE_ID…

MarcoR83 opened a new pull request, #4040:
URL: https://github.com/apache/activemq-artemis/pull/4040

   This fixes issues after upgrading from older version (2.19.0 in my case) to newest release 2.21.0.
   
   Existing messages could not be deliverd to clients using the OpenWire protocol. Server gave following error logs:
   ```
   [org.apache.activemq.artemis.core.server] AMQ222302: Failed to deal with property __HDR_PRODUCER_ID when converting message from core to OpenWire: Cannot cast [B to org.apache.activemq.artemis.api.core.SimpleString       │
   [org.apache.activemq.artemis.core.server] AMQ222302: Failed to deal with property __HDR_MESSAGE_ID when converting message from core to OpenWire: Cannot cast [B to org.apache.activemq.artemis.api.core.SimpleString  
   ```  


-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] jbertram commented on pull request #4040: OpenWireMessageConverter - Allow binary deserialization of existing message properties

Posted by GitBox <gi...@apache.org>.
jbertram commented on PR #4040:
URL: https://github.com/apache/activemq-artemis/pull/4040#issuecomment-1106808773

   @MarcoR83, checkout https://github.com/jbertram/activemq-artemis/commit/f44b1de555d0ea8da91706f57573fa754a549e2d. It should cover your 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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] asfgit closed pull request #4040: OpenWireMessageConverter - Allow binary deserialization of existing message properties

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #4040: OpenWireMessageConverter - Allow binary deserialization of existing message properties
URL: https://github.com/apache/activemq-artemis/pull/4040


-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] clebertsuconic commented on pull request #4040: OpenWireMessageConverter - Allow binary deserialization of existing message properties

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on PR #4040:
URL: https://github.com/apache/activemq-artemis/pull/4040#issuecomment-1108662123

   @MarcoR83 I merged it with some git-fu.. if you could take a look please?


-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] clebertsuconic commented on pull request #4040: Allow binary deserialization of message properties AMQ_MSG_MESSAGE_ID…

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on PR #4040:
URL: https://github.com/apache/activemq-artemis/pull/4040#issuecomment-1106578527

   can you try against upstream/main first?
   
   @AntonRoskvist  recently added 32c5f9d2685252e4b5fb213527084173351d7d63
   
   
   wouldn't your issue be handled by this?


-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] MarcoR83 commented on pull request #4040: OpenWireMessageConverter - Allow binary deserialization of existing message properties

Posted by GitBox <gi...@apache.org>.
MarcoR83 commented on PR #4040:
URL: https://github.com/apache/activemq-artemis/pull/4040#issuecomment-1108186352

   Alrighty. Will pull this test into my branch then and see what it does. Will also create that Jira-Item and adapt the commit message


-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] clebertsuconic commented on pull request #4040: Allow binary deserialization of message properties AMQ_MSG_MESSAGE_ID…

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on PR #4040:
URL: https://github.com/apache/activemq-artemis/pull/4040#issuecomment-1106637668

   Well.. without tests it will be impossible to consider merging this... I'm not sure if this is something too specific for your use case or something that would be part of this branch.


-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] jbertram commented on pull request #4040: Allow binary deserialization of message properties AMQ_MSG_MESSAGE_ID…

Posted by GitBox <gi...@apache.org>.
jbertram commented on PR #4040:
URL: https://github.com/apache/activemq-artemis/pull/4040#issuecomment-1106626124

   Can you clarify what you mean by, "could not be deliverd"? The problem is logged as a WARN and the message _should_ continue to be dispatched to the client albeit without the properties referenced in the log.


-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] jbertram commented on pull request #4040: OpenWireMessageConverter - Allow binary deserialization of existing message properties

Posted by GitBox <gi...@apache.org>.
jbertram commented on PR #4040:
URL: https://github.com/apache/activemq-artemis/pull/4040#issuecomment-1106688943

   > So if those properties are not needed by the OpenWire clients this could be ignored, but I feel it is still not the greatest way just to ignore. The data was written with an older version and now it is not recognized anymore. 
   
   I didn't mean to imply that we ignore the warning messages (although it's safe to ignore in almost all circumstances). I was simply trying to understand your statement that the message "could not be deliverd" as that did not fit with my understanding of that code.
   
   > IMHO bad practice for a data broker.
   
   Agreed. This was an oversight in the changes from [ARTEMIS-3698](https://issues.apache.org/jira/browse/ARTEMIS-3698).
   
   > So I can/should create an Jira issue? 
   
   Yes.
   
   > Anything I need to consider when filling out the fields?
   
   No. Just fill out the title and the description. The defaults should be fine for everything else.
   
   Thanks!
   


-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] clebertsuconic commented on pull request #4040: Allow binary deserialization of message properties AMQ_MSG_MESSAGE_ID…

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on PR #4040:
URL: https://github.com/apache/activemq-artemis/pull/4040#issuecomment-1106591494

   we would need a test as part of your PR as well... + a JIRA + amend your commit message with your JIRA.


-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] MarcoR83 commented on pull request #4040: Allow binary deserialization of message properties AMQ_MSG_MESSAGE_ID…

Posted by GitBox <gi...@apache.org>.
MarcoR83 commented on PR #4040:
URL: https://github.com/apache/activemq-artemis/pull/4040#issuecomment-1106652342

   Alright. Ok  so lets see... 
   
   1. @AntonRoskvist recently added https://github.com/apache/activemq-artemis/commit/32c5f9d2685252e4b5fb213527084173351d7d63 I had a look into it and it does not solve the warning (it is used on inbound OpenWire Messages)
   
   2. Can you clarify what you mean by, "could not be deliverd"? 
   Ok I checked that error handling. True it does not stop the code from proceeding, but the prior existing property gets lost along the way. I dont know what this might imply, as I'm not super into all of this. I just saw the error popping up after upstaging 2.21.0 on our environments. So if this properties are not needed by the OpenWire Clients this could be ignored, but I feel it is still not the greatest way just to ignore. The data was written with an older version and now it wont be recognized anymore. IMHO bad practise for a data broker.
   
   3. Well.. without tests it will be impossible to consider merging this... 
   I will hopefully be able to create  suitable testcases. Would have so see to store Properties the old way and then read them using OpenWire protocol.. Not sure where to start... will have to dive deeper into the code/tests. Will come back with an update.
   
   4.  a JIRA + amend your commit message with your JIRA. 
   So I can/should create an Jira issue? Anything I need to consider when filling out the fields?
   Amend the commit/s with the Jira-key should not be a problem ;) 
   
   
   


-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] jbertram commented on pull request #4040: OpenWireMessageConverter - Allow binary deserialization of existing message properties

Posted by GitBox <gi...@apache.org>.
jbertram commented on PR #4040:
URL: https://github.com/apache/activemq-artemis/pull/4040#issuecomment-1106696160

   > Not sure where to start... will have to dive deeper into the code/tests. Will come back with an update.
   
   I am going to look at creating a test for this as well since it was my commit that broke it in the first place. I expect that it can be done with some carefully crafted messages in a unit test. Stay tuned.


-- 
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: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org