[GitHub] qpid-proton-j issue #6: PROTON-1410 Moving ThreadLocal logic for Decoders on...
Github user clebertsuconic commented on the issue: https://github.com/apache/qpid-proton-j/pull/6 I can live without this one. The other with the delivery size would be needed for me. I will rework that one and close this one. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j issue #6: PROTON-1410 Moving ThreadLocal logic for Decoders on...
Github user tabish121 commented on the issue: https://github.com/apache/qpid-proton-j/pull/6 Personally this doesn't seem a necessary addition to me. I prefer to keep the management of the encoder and decoder instances in the Qpid JMS code and wouldn't use this myself. It adds support code that then has to be maintained as part of the proton-j code that just means we have to manage it later if the Encoders were say made to not require being managed as thread locals for instance, e.g. we made them stateless --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j issue #6: PROTON-1410 Moving ThreadLocal logic for Decoders on...
Github user clebertsuconic commented on the issue: https://github.com/apache/qpid-proton-j/pull/6 @gemmellr I am doing the same thing... avoiding using MessageImpl from Proton. I just feel weird having to do a copy & paste inheritance every time someone needs that functionality and it would be better on some utility class to be reused. It's just an utility class, I can rename to any name you like. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j issue #6: PROTON-1410 Moving ThreadLocal logic for Decoders on...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/6 I'm aware of what qpid-jms does, it does that while avoiding using the Message[Impl] from proton entirely, meaning it isn't using the thread local from there either, but just happens to have its own (though it could actually do the same without that thread local if it wanted, given how the rest of the client works.) I wouldnt be all that inclined to make qpid-jms use the proposed TLSEncoder class even if it already existed in proton, it seems nicer to keep that in the client. As an aside, TLS seems a weird acronym to use in the name, given its typical meaning/usage. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j issue #6: PROTON-1410 Moving ThreadLocal logic for Decoders on...
Github user clebertsuconic commented on the issue: https://github.com/apache/qpid-proton-j/pull/6 ```a third bit of code stikes me as ugly,``` - as I said the best I envision is not not need multiple instances. Meanwhile what do you suggest? duplicating the code and the instances as we are doing now? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j issue #6: PROTON-1410 Moving ThreadLocal logic for Decoders on...
Github user clebertsuconic commented on the issue: https://github.com/apache/qpid-proton-j/pull/6 Both qpid-jms and artemis are holding this type of TLS somewhere else. I am creating my own version of an object that is only holding the byte[] of the message, and I only need it to decode and re-encode headers and application properties. - https://github.com/apache/qpid-jms/blob/master/qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/message/AmqpCodec.java#L61-L76 - https://github.com/apache/activemq-artemis/blob/master/artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/converter/message/JMSMappingOutboundTransformer.java#L109-L124 My final proposal would be to have it singleton, but meanwhile I still need that functionality.. I think it would be better to be reusing the same TLS that's already used inside ProtonJMS instead of duplicating the variable. I am expanding the usage to a new AMQPMessage on my branch, and it's using a delegate approach where I will try to not load the Message, and only parse the header and eventually applicationProperties: --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j issue #6: PROTON-1410 Moving ThreadLocal logic for Decoders on...
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/6 Can you elaborate a little on what you are doing to need this? Are you just subclassing the MessageImpl? If so would protected getter(s) work for you? Creating a separate new 'non impl' utility class purely to access a thread local of a specific implementation from within a third bit of code stikes me as ugly, especially if it is already a subclass of the starting point. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[GitHub] qpid-proton-j issue #6: PROTON-1410 Moving ThreadLocal logic for Decoders on...
Github user clebertsuconic commented on the issue: https://github.com/apache/qpid-proton-j/pull/6 This supersedes #5 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
