[GitHub] qpid-proton-j issue #20: PROTON-1965 Optimize CompositeReadableBuffer::equal...

2018-11-21 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/20 For later reference, a related further change was made via b3a66b741c34b40b059fc023b3c7650720dfc001 to apply the optimisation in more cases

[GitHub] qpid-proton-j issue #20: PROTON-1965 Optimize CompositeReadableBuffer::equal...

2018-11-20 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/20 I've pushed an updated version of the change with a fix for the offset issue I noticed yesterday, which Tim added a test to cover

[GitHub] qpid-proton-j pull request #20: PROTON-1965 Optimize CompositeReadableBuffer...

2018-11-19 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-proton-j/pull/20#discussion_r234714952 --- Diff: proton-j/src/main/java/org/apache/qpid/proton/codec/CompositeReadableBuffer.java --- @@ -834,22 +834,39 @@ public boolean equals(Object

[GitHub] qpid-proton-j issue #20: PROTON-1965 Optimize CompositeReadableBuffer::equal...

2018-11-19 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/20 Can you unwind the 'revert' of my currentPos -> origPos variable name change from earlier? :) --- - To unsubscribe

[GitHub] qpid-proton-j issue #20: PROTON-1965 Optimize CompositeReadableBuffer::equal...

2018-11-19 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/20 Checked in with infra who looked into it and found the issue, mirror is now back up to date. --- - To unsubscribe, e-mail

[GitHub] qpid-proton-j issue #20: PROTON-1965 Optimize CompositeReadableBuffer::equal...

2018-11-19 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/20 It seems the github mirror is not up to date, and I've since prodded a re-sync and it still isn't up to date. I'll possibly need to ask infra about it after lunch

[GitHub] qpid-proton-j issue #20: PROTON-1965 Optimize CompositeReadableBuffer::equal...

2018-11-19 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/20 The reset is a good catch. Its a noteworthy bug in its own right and so should be fixed separately, and also tested. I have done that now via PROTON-1966. Sorry for the hassle @franz1981

[GitHub] qpid-jms issue #25: [QPIDJMS-426] Update to proton-j 0.30.0

2018-11-12 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-jms/pull/25 Sorry, I didn't notice you raised this. I had the change sitting ready since preparing 0.30.0 last week, awaiting the vote and then completing the announcement work. It never occurred to me anyone

[GitHub] qpid-proton-j issue #19: PROTON-1958 Fix use of AtomicInteger for instance I...

2018-11-06 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/19 Can you please close out this PR? I went with a different approach, committed in 83609c4752fbdaa1ddfa032285e5caa09b61f480, of making each reactor timer maintain a counter for its tasks

[GitHub] qpid-proton-j issue #15: PROTON-1916: Makes StringsBenchmark::encodeStringMe...

2018-11-06 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/15 We don't get notified of updated pushes for the mirrors, so I didn't know you had modified this since my comment

[GitHub] qpid-proton-j pull request #18: NO-JIRA Minor code improvements (e.g. semico...

2018-11-06 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-proton-j/pull/18#discussion_r231104760 --- Diff: proton-j/src/main/java/org/apache/qpid/proton/message/impl/MessageImpl.java --- @@ -49,11 +49,7

[GitHub] qpid-proton-j issue #16: PROTON-1938: Fix error propagation in TransportImpl

2018-10-30 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/16 As noted on the JIRA, an alternate change was made in 19bf22f7e9b88e73db4b195f674d1527dbd713f3. --- - To unsubscribe, e

[GitHub] qpid-proton-j issue #18: NO-JIRA Minor code improvements (e.g. semicolons an...

2018-10-16 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/18 If the changes aren't related enough to be in the same commit, they probably shouldn't be in the same PR to begin with :) (My view is that nothing but the most complex layered changes

[GitHub] qpid-proton-j pull request #18: NO-JIRA Minor code improvements (e.g. semico...

2018-10-16 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-proton-j/pull/18#discussion_r225562577 --- Diff: examples/reactor/src/main/java/org/apache/qpid/proton/example/reactor/EchoInputStreamWrapper.java --- @@ -31,11 +31,12

[GitHub] qpid-proton-j issue #19: PROTON-1958 Fix use of AtomicInteger for instance I...

2018-10-16 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/19 I'll merge this later, could perhaps do with a test. Note that the example change should really have been kept separate as it has nothing to do with the defect (and as noted before

[GitHub] qpid-proton-j pull request #18: NO-JIRA fix use of AtomicInteger and other v...

2018-10-15 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-proton-j/pull/18#discussion_r225224724 --- Diff: examples/reactor/src/main/java/org/apache/qpid/proton/example/reactor/EchoInputStreamWrapper.java --- @@ -31,11 +31,12

[GitHub] qpid-proton-j pull request #18: NO-JIRA fix use of AtomicInteger and other v...

2018-10-15 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-proton-j/pull/18#discussion_r225236948 --- Diff: proton-j/src/main/java/org/apache/qpid/proton/reactor/impl/TaskImpl.java --- @@ -29,10 +29,12 @@ import

[GitHub] qpid-proton-j pull request #18: NO-JIRA fix use of AtomicInteger and other v...

2018-10-15 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-proton-j/pull/18#discussion_r225223601 --- Diff: proton-j/src/main/java/org/apache/qpid/proton/engine/impl/ssl/DefaultSslEngineFacade.java --- @@ -36,7 +36,7

[GitHub] qpid-jms issue #22: QPIDJMS-417 Reduce GC pressure while using BytesMessage

2018-10-10 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-jms/pull/22 Can you elaborate on the benefits you measured here? I'd like to understand the extent to consider against the downside of exposing dep impl types throughout the code base. The existing

[GitHub] qpid-proton-j issue #15: PROTON-1916: Makes StringsBenchmark::encodeStringMe...

2018-08-22 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/15 The JIRA, PR, and commit say they are primarily changing the benchmark/test but most of the change has nothing to do with that. Even though the perf change is tiny and most things wont see

[GitHub] qpid-proton-j issue #14: PROTON-1911: Improve performance of EncoderImpl#wri...

2018-08-20 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/14 After some investigation and testing, a different approach (https://github.com/apache/qpid-proton-j/commit/7eac8b945c8ce90f091126d34cf174e8792fdfc0) for improvement was taken as discussed

[GitHub] qpid-proton issue #133: PROTON-1796 branch for automated periodic Coverity S...

2018-07-26 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton/pull/133 I meant the PR probably doesnt make sense, i.e we wouldnt have a branch here, which it seems you agree

[GitHub] qpid-proton issue #133: PROTON-1796 branch for automated periodic Coverity S...

2018-07-26 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton/pull/133 I don't think we have ability to configure periodic builds in Travis on this mirror, over which we have limited control in which case this doesn't really make sense

[GitHub] qpid-jms issue #20: Issue QPIDJMS-400: Add missing OSGI resolution:=optional...

2018-07-12 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-jms/pull/20 That wont actually work since the epoll bits are required by default, as the classes are used by default, in determining whether the functionality is currently available

[GitHub] qpid-jms issue #20: Issue QPIDJMS-400: Add missing OSGI resolution:=optional...

2018-07-12 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-jms/pull/20 > FYI, attempting to exclude the transitive dependency on the kqueue jar so only the epoll jar is available in the bundle does not fix the problem, the OSGI container again complains ab

[GitHub] qpid-jms issue #20: Issue QPIDJMS-400: Add missing OSGI resolution:=optional...

2018-07-12 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-jms/pull/20 I agree the Travis CI OSX failure is unrelated. There isn't a way to trigger a new build on the github mirror without updating the PR. I don't think this change is actually quite the way

[GitHub] qpid-proton issue #142: PROTON-1831: Incorrect open/close sequence for same ...

2018-05-02 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton/pull/142 It seems you already committed this before Gordon and I commented, but neglected to close the PR with the commit or otherwise. It either needs closing, or the commit reverted, depending what

[GitHub] qpid-proton issue #142: PROTON-1831: Incorrect open/close sequence for same ...

2018-04-27 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton/pull/142 > Thanks - I didn't know about stolen. I think we could implement what the spec says on the server side - close the old link/handle with "stolen" and create a new link under t

[GitHub] qpid-proton issue #142: PROTON-1831: Incorrect open/close sequence for same ...

2018-04-27 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton/pull/142 The use of "invalid-field" for the error feels like it is maybe a little off. Its possible/likely there is nothing wrong with the information in the fields of the attac

[GitHub] qpid-cpp issue #12: WIP - A batch of C++ updates

2018-03-13 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-cpp/pull/12 Docs, removing 'untested proton version' warning, etc, changes look good to me. I'll trust Gordon about the code

[GitHub] qpid-proton issue #140: PROTON-1638, PROTON-1728: Reorganize the source tree

2018-03-12 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton/pull/140 I gave things a skim over, changes seemed reasonable from my doesnt-do-cmake-etc perspective. Noting for later reference: this builds on / incorporates WIP from #136 and #138

[GitHub] qpid-dispatch issue #252: Tross dispatch 845 1

2018-01-31 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-dispatch/pull/252 Couple of questions. Probably a thing to consider even with the existing config method, but how are routes treated when more than one connection says it matches something

[GitHub] qpid-proton-j issue #13: Added API to Transport interface to allow custom sa...

2018-01-09 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/13 For completeness, a different change was made via https://issues.apache.org/jira/browse/PROTON-1736 in commit 17cef9ace9a7c75901d517f951ae1d4610819436

[GitHub] qpid-proton-j issue #13: Added API to Transport interface to allow custom sa...

2017-12-19 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/13 My immediate reaction is that this isn't acceptable. It exposes various APIs that are considered part of the implementation only, and then further exposes additional implementation detail

[GitHub] qpid-proton-j issue #12: PROTON-1690 JMH Benchmarks for baseline performance...

2017-11-24 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/12 Hi Franz Thanks for the PR. I have some feedback for changes that would be good and/or necessary. The JMH dependencies are not ALv2 compatible and so I think we should make

[GitHub] qpid-proton issue #129: PROTON-522: Apache Qpid Proton on Mac/OSX - C/Object...

2017-11-23 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton/pull/129 @astitcher seems to have made various changes related to these ones in commits such as: d82bbfab037c97e1c403ae701f1b3fe272813ff7 , b6ad8a996faa34aeb8e475902a6f151c5476d45f

[GitHub] qpid-proton issue #128: PROTON-1342: CI on OS X

2017-11-03 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton/pull/128 I agree on reducing the number of builds. The /apache org has a shared job capacity for all the foundations projects, so we dont want to be overloading things, and tripling the number just

[GitHub] qpid-dispatch issue #212: DISPATCH-858 - Moved third party text into files a...

2017-10-20 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-dispatch/pull/212 @ganeshmurthy looks good --- - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail

[GitHub] qpid-cpp issue #8: QPID-7874 Use qpid-route quiet option to suppress extra l...

2017-09-29 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-cpp/pull/8 @chrisrichardson77 A change for this looks to have been made in https://git-wip-us.apache.org/repos/asf?p=qpid-cpp.git;h=55d4171, can you close the PR please? (The commit log message

[GitHub] qpid-cpp issue #7: QPID-7876 qpid-route does not properly consider src-local...

2017-09-29 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-cpp/pull/7 @chrisrichardson77 A change for this looks to have been made in https://git-wip-us.apache.org/repos/asf?p=qpid-cpp.git;a=commitdiff;h=edccbc9e1737603a1d1f66f0df8499dbba07e93b, can you close the PR

[GitHub] qpid-cpp issue #6: QPID-7875 qpid-route fetches links multiple times when de...

2017-09-29 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-cpp/pull/6 @chrisrichardson77 A change for this looks to have been made in https://git-wip-us.apache.org/repos/asf?p=qpid-cpp.git;a=commitdiff;h=a8d392efe30ef763d2e93c6ce733976ac786c0f0, can you close the PR

[GitHub] qpid-cpp issue #4: QPID-7702: Fix some minor memory leaks detected by Coveri...

2017-09-29 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-cpp/pull/4 @kgiusti Can you close the PR? This looks to have been applied in https://git-wip-us.apache.org/repos/asf?p=qpid-cpp.git;h=f91a23c

[GitHub] qpid-proton-j issue #10: PROTON-1551: Fix the problem with binary content ov...

2017-08-30 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/10 Can you please squash the changes into a single commit and make the commit message a little more specific, e.g "PROTON-1551: fix length encoding for binary over 255 bytes in length when

[GitHub] qpid-jms issue #12: QPIDJMS-315: Add support for Netty KQueue transport

2017-08-25 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-jms/pull/12 > Agreed its not ideal we have to do this, but this was found on epoll in artemis (which you already have epoll implemented and defaulted to true) , as such until netty improve the ch

[GitHub] qpid-jms issue #12: QPIDJMS-315: Add support for Netty KQueue transport

2017-08-25 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-jms/pull/12 I don't think the various environment etc checking being done really belongs in the client, it seems like that stuff belongs in Netty if the isAvailable() checks it offers aren't actually

[GitHub] qpid-dispatch issue #190: DISPATCH-802 - Additional fixes - 1. Modified erro...

2017-08-24 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-dispatch/pull/190 Tested the latest version and the attach target is now null as needed to indicate refusal, and the updated error details look good. --- If your project is set up for it, you can reply

[GitHub] qpid-jms pull request #10: [QPIDJMS-303] implement sasl gssapi (kerberos) me...

2017-07-21 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-jms/pull/10#discussion_r128809457 --- Diff: qpid-jms-client/src/test/java/org/apache/qpid/jms/test/testpeer/TestAmqpPeer.java --- @@ -495,6 +512,169 @@ public void run

[GitHub] qpid-jms pull request #10: [QPIDJMS-303] implement sasl gssapi (kerberos) me...

2017-07-21 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-jms/pull/10#discussion_r128810019 --- Diff: qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/AmqpSaslAuthenticator.java --- @@ -137,7 +142,9 @@ private void handleSaslStep

[GitHub] qpid-jms pull request #10: [QPIDJMS-303] implement sasl gssapi (kerberos) me...

2017-07-21 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-jms/pull/10#discussion_r128807526 --- Diff: qpid-jms-client/src/main/java/org/apache/qpid/jms/sasl/GssapiMechanism.java --- @@ -0,0 +1,190 @@ +/* + * Licensed to the Apache

[GitHub] qpid-jms pull request #10: [QPIDJMS-303] implement sasl gssapi (kerberos) me...

2017-07-21 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-jms/pull/10#discussion_r128808271 --- Diff: qpid-jms-client/src/main/java/org/apache/qpid/jms/sasl/GssapiMechanism.java --- @@ -0,0 +1,190 @@ +/* + * Licensed to the Apache

[GitHub] qpid-jms pull request #10: [QPIDJMS-303] implement sasl gssapi (kerberos) me...

2017-07-21 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-jms/pull/10#discussion_r128755403 --- Diff: qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/SaslGssApiIntegrationTest.java --- @@ -0,0 +1,185

[GitHub] qpid-jms pull request #10: [QPIDJMS-303] implement sasl gssapi (kerberos) me...

2017-07-21 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-jms/pull/10#discussion_r128747741 --- Diff: qpid-jms-client/src/main/resources/login.config --- @@ -0,0 +1,23 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one

[GitHub] qpid-jms pull request #10: [QPIDJMS-303] implement sasl gssapi (kerberos) me...

2017-07-21 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-jms/pull/10#discussion_r128750540 --- Diff: qpid-jms-client/src/test/java/org/apache/qpid/jms/test/testpeer/TestAmqpPeer.java --- @@ -495,6 +512,169 @@ public void run

[GitHub] qpid-jms pull request #10: [QPIDJMS-303] implement sasl gssapi (kerberos) me...

2017-07-21 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-jms/pull/10#discussion_r128754495 --- Diff: qpid-jms-client/src/test/java/org/apache/qpid/jms/test/testpeer/TestAmqpPeer.java --- @@ -495,6 +512,169 @@ public void run

[GitHub] qpid-jms pull request #10: [QPIDJMS-303] implement sasl gssapi (kerberos) me...

2017-07-21 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-jms/pull/10#discussion_r128754756 --- Diff: qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/SaslGssApiIntegrationTest.java --- @@ -0,0 +1,185

[GitHub] qpid-jms pull request #10: [QPIDJMS-303] implement sasl gssapi (kerberos) me...

2017-07-21 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-jms/pull/10#discussion_r128754720 --- Diff: qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/SaslGssApiIntegrationTest.java --- @@ -0,0 +1,185

[GitHub] qpid-jms pull request #10: [QPIDJMS-303] implement sasl gssapi (kerberos) me...

2017-07-21 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-jms/pull/10#discussion_r128749340 --- Diff: qpid-jms-client/src/test/java/org/apache/qpid/jms/test/testpeer/TestAmqpPeer.java --- @@ -495,6 +512,169 @@ public void run

[GitHub] qpid-jms pull request #10: [QPIDJMS-303] implement sasl gssapi (kerberos) me...

2017-07-21 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-jms/pull/10#discussion_r128727012 --- Diff: qpid-jms-client/pom.xml --- @@ -93,6 +93,19 @@ hamcrest-all test + + org.apache.hadoop

[GitHub] qpid-jms pull request #10: [QPIDJMS-303] implement sasl gssapi (kerberos) me...

2017-07-21 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-jms/pull/10#discussion_r128752547 --- Diff: qpid-jms-client/src/test/resources/minikdc-krb5.conf --- @@ -0,0 +1,26 @@ +# +# Licensed to the Apache Software Foundation (ASF) under

[GitHub] qpid-jms pull request #10: [QPIDJMS-303] implement sasl gssapi (kerberos) me...

2017-07-21 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-jms/pull/10#discussion_r128726699 --- Diff: qpid-jms-client/src/main/java/org/apache/qpid/jms/provider/amqp/AmqpSaslAuthenticator.java --- @@ -137,7 +142,9 @@ private void handleSaslStep

[GitHub] qpid-jms pull request #10: [QPIDJMS-303] implement sasl gssapi (kerberos) me...

2017-07-21 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-jms/pull/10#discussion_r128749066 --- Diff: qpid-jms-client/src/test/java/org/apache/qpid/jms/integration/SaslIntegrationTest.java --- @@ -54,6 +54,7 @@ private static final

[GitHub] qpid-jms pull request #10: [QPIDJMS-303] implement sasl gssapi (kerberos) me...

2017-07-21 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-jms/pull/10#discussion_r128748789 --- Diff: qpid-jms-client/src/main/java/org/apache/qpid/jms/sasl/GssapiMechanism.java --- @@ -0,0 +1,163 @@ +/* + * Licensed to the Apache

[GitHub] qpid-jms pull request #10: [QPIDJMS-303] implement sasl gssapi (kerberos) me...

2017-07-21 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-jms/pull/10#discussion_r128747005 --- Diff: qpid-jms-client/src/main/java/org/apache/qpid/jms/sasl/GssapiMechanism.java --- @@ -0,0 +1,163 @@ +/* + * Licensed to the Apache

[GitHub] qpid-jms issue #9: QPIDJMS-294: Ensure that SASL mechanism has completed bef...

2017-07-18 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-jms/pull/9 Looks good. I gave things a try with the changes from PROTON-1486 (now pushed) against Dispatch and the C++ broker, which continue to send the explicit challange before the outcome rather than use

[GitHub] qpid-jms issue #7: Client+Server class additions - updated 5/15/17

2017-05-17 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-jms/pull/7 Hi Austin, I've squashed your chagnes into a single commit (https://github.com/apache/qpid-jms/commit/72c7bb0880b5cc7dd332c0220091f93d0de9a5a2) and put it in along with further commit

[GitHub] qpid-jms pull request #7: Client+Server class additions - updated

2017-05-11 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-jms/pull/7#discussion_r116052679 --- Diff: qpid-jms-examples/src/main/java/org/apache/qpid/jms/example/Client.java --- @@ -0,0 +1,106 @@ +/* + * + * Licensed to the Apache

[GitHub] qpid-jms pull request #7: Client+Server class additions - updated

2017-05-11 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-jms/pull/7#discussion_r116064745 --- Diff: qpid-jms-examples/src/main/java/org/apache/qpid/jms/example/Server.java --- @@ -0,0 +1,94 @@ +/* + * + * Licensed to the Apache

[GitHub] qpid-jms pull request #7: Client+Server class additions - updated

2017-05-11 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-jms/pull/7#discussion_r116048356 --- Diff: qpid-jms-examples/src/main/java/org/apache/qpid/jms/example/Client.java --- @@ -0,0 +1,106 @@ +/* + * + * Licensed to the Apache

[GitHub] qpid-jms pull request #7: Client+Server class additions - updated

2017-05-11 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-jms/pull/7#discussion_r116053494 --- Diff: qpid-jms-examples/src/main/java/org/apache/qpid/jms/example/Client.java --- @@ -0,0 +1,106 @@ +/* + * + * Licensed to the Apache

[GitHub] qpid-jms pull request #7: Client+Server class additions - updated

2017-05-11 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-jms/pull/7#discussion_r116057472 --- Diff: qpid-jms-examples/src/main/java/org/apache/qpid/jms/example/Client.java --- @@ -0,0 +1,106 @@ +/* + * + * Licensed to the Apache

[GitHub] qpid-jms pull request #7: Client+Server class additions - updated

2017-05-11 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-jms/pull/7#discussion_r116069936 --- Diff: qpid-jms-examples/src/main/java/org/apache/qpid/jms/example/Server.java --- @@ -0,0 +1,94 @@ +/* + * + * Licensed to the Apache

[GitHub] qpid-jms pull request #6: Added two new example classes, Client and Server.

2017-05-09 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-jms/pull/6#discussion_r115542360 --- Diff: qpid-jms-examples/src/main/java/org/apache/qpid/jms/example/Server.java --- @@ -0,0 +1,119 @@ +/* + * + * Licensed to the Apache

[GitHub] qpid-jms pull request #6: Added two new example classes, Client and Server.

2017-05-09 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-jms/pull/6#discussion_r115541734 --- Diff: qpid-jms-examples/src/main/java/org/apache/qpid/jms/example/Server.java --- @@ -0,0 +1,119 @@ +/* + * + * Licensed to the Apache

[GitHub] qpid-jms pull request #6: Added two new example classes, Client and Server.

2017-05-09 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-jms/pull/6#discussion_r115540310 --- Diff: qpid-jms-examples/src/main/java/org/apache/qpid/jms/example/Server.java --- @@ -0,0 +1,119 @@ +/* + * + * Licensed to the Apache

[GitHub] qpid-jms pull request #6: Added two new example classes, Client and Server.

2017-05-09 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-jms/pull/6#discussion_r115537587 --- Diff: qpid-jms-examples/src/main/java/org/apache/qpid/jms/example/Client.java --- @@ -0,0 +1,204 @@ + +package org.apache.qpid.jms.example

[GitHub] qpid-jms pull request #6: Added two new example classes, Client and Server.

2017-05-09 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-jms/pull/6#discussion_r115537215 --- Diff: qpid-jms-examples/src/main/java/org/apache/qpid/jms/example/Client.java --- @@ -0,0 +1,204 @@ + --- End diff -- This new file

[GitHub] qpid-jms pull request #6: Added two new example classes, Client and Server.

2017-05-09 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-jms/pull/6#discussion_r115538757 --- Diff: qpid-jms-examples/src/main/java/org/apache/qpid/jms/example/Client.java --- @@ -0,0 +1,204 @@ + +package org.apache.qpid.jms.example

[GitHub] qpid-jms pull request #6: Added two new example classes, Client and Server.

2017-05-09 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-jms/pull/6#discussion_r115539336 --- Diff: qpid-jms-examples/src/main/java/org/apache/qpid/jms/example/Client.java --- @@ -0,0 +1,204 @@ + +package org.apache.qpid.jms.example

[GitHub] qpid-jms pull request #6: Added two new example classes, Client and Server.

2017-05-09 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-jms/pull/6#discussion_r115540862 --- Diff: qpid-jms-examples/src/main/java/org/apache/qpid/jms/example/Server.java --- @@ -0,0 +1,119 @@ +/* + * + * Licensed to the Apache

[GitHub] qpid-jms issue #6: Added two new example classes, Client and Server.

2017-05-09 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-jms/pull/6 Hi Austin, I think this is a good start. There are some changes that I think would improve the new example classes, in general I think the classes would benefit from some simplification

[GitHub] qpid-proton issue #102: NO-JIRA: revert temporary workaround for PROTON-1453

2017-04-13 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton/pull/102 Looks like the change was applied, but was no longer the head commit at the time, and didn't have "This closes #102" in the message of the commit itself or a merge commit. Can

[GitHub] qpid-proton-j issue #8: 0.16.x

2017-03-08 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/8 Did you raise this in error? If so please close it. --- 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

[GitHub] qpid-proton-j issue #7: PROTON-1409 Exposing delivery length

2017-03-06 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/7 I've pushed a change to expose the currently available bytes length. I went with 'available()' since it felt natural for what its indicating, and applying to both send/recv cases. In hindsight

[GitHub] qpid-proton-j issue #6: PROTON-1410 Moving ThreadLocal logic for Decoders on...

2017-03-01 Thread gemmellr
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

[GitHub] qpid-proton-j issue #6: PROTON-1410 Moving ThreadLocal logic for Decoders on...

2017-03-01 Thread gemmellr
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

[GitHub] qpid-proton-j issue #7: PROTON-1409 Exposing delivery length

2017-03-01 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/7 The javadoc could do with making a little clearer that it returns the currently unread length of the existing payload for the delivery, and is not the length of the delivery (which might

[GitHub] qpid-proton-j issue #4: SSLContext changes

2017-02-17 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/4 Actually, if you could also confirm if its just working for you that would be good too, as I can then look to kick off the release process on 0.18.0 some time, perhaps next week. --- If your

[GitHub] qpid-proton-j issue #4: SSLContext changes

2017-02-17 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/4 I've pushed an updated set of changes, to address some of my earlier feedback and some issues with the subsequent commit. Give things a try out on master and comment here or on PROTON-1405

[GitHub] qpid-proton-j issue #4: SSLContext changes

2017-02-14 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/4 I've had a look and have some comments: - The changes should inclulde some tests for the new feature. - The setter javadoc should make clear that this overrides any other configuration

[GitHub] qpid-proton-j issue #2: Delivery link leak

2017-02-13 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/2 I've raised #3 with some alternative changes, which are essentially just a subset of yours with a different test. One of the changes here would corrupt the linkPrev/next entries chain, which

[GitHub] qpid-proton-j pull request #3: PROTON-1393: fully sever delivery refs during...

2017-02-13 Thread gemmellr
GitHub user gemmellr opened a pull request: https://github.com/apache/qpid-proton-j/pull/3 PROTON-1393: fully sever delivery refs during removal Fully sever delivery references for related 'lists' during removal to prevent unexpected retention of old deliveries that arent otherwise

[GitHub] qpid-proton-j issue #2: Delivery link leak

2017-02-09 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/2 I've had a closer look at this now, sorry it took so long. I'm not sure about some of the changes, I'm going to give it a furhter look tomorrow to either establish I'm wrong, or suggest some

[GitHub] qpid-java issue #1: upgrading logback to latest 1.1.7

2017-02-09 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-java/pull/1 Aligns to https://issues.apache.org/jira/browse/QPID-7468 --- 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

[GitHub] qpid-proton-j issue #1: Memory leak with delayed settlement

2017-01-25 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton-j/pull/1 I am in a F2F meeting (or travelling to/from it) all this week so I probably won't get a chance to look closely until next week, but I took a skim of this. I can see from the changes

[GitHub] qpid-proton issue #89: [PROTON-1352] Trivial casting from UnsignedByte to Se...

2016-11-16 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton/pull/89 I hadn't realised you had done separate commits...can you add the relevant JIRA key to the first commit, then squash the other two together as a single commit and give it the other key

[GitHub] qpid-proton issue #89: [PROTON-1352] Trivial casting from UnsignedByte to Se...

2016-11-16 Thread gemmellr
Github user gemmellr commented on the issue: https://github.com/apache/qpid-proton/pull/89 Including a message as to why the IAE is thrown wouldn't go amiss :) I think you missed my comment about raising a separate JIRA for the unrelated annotations change, then including its

[GitHub] qpid-proton pull request #89: [PROTON-1352] Trivial casting from UnsignedByt...

2016-11-16 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-proton/pull/89#discussion_r88262982 --- Diff: proton-j/src/main/java/org/apache/qpid/proton/amqp/transport/ReceiverSettleMode.java --- @@ -25,13 +25,32 @@ import

[GitHub] qpid-proton pull request #89: [PROTON-1352] Trivial casting from UnsignedByt...

2016-11-16 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-proton/pull/89#discussion_r88259929 --- Diff: proton-j/src/main/java/org/apache/qpid/proton/amqp/transport/ReceiverSettleMode.java --- @@ -25,13 +25,32 @@ import

[GitHub] qpid-proton pull request #89: [PROTON-1352] Trivial casting from UnsignedByt...

2016-11-16 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-proton/pull/89#discussion_r88247744 --- Diff: proton-j/src/main/java/org/apache/qpid/proton/amqp/transport/ReceiverSettleMode.java --- @@ -25,13 +25,32 @@ import

[GitHub] qpid-proton pull request #89: [PROTON-1352] Trivial casting from UnsignedByt...

2016-11-16 Thread gemmellr
Github user gemmellr commented on a diff in the pull request: https://github.com/apache/qpid-proton/pull/89#discussion_r88248771 --- Diff: proton-j/src/main/java/org/apache/qpid/proton/amqp/transport/SenderSettleMode.java --- @@ -25,13 +25,33 @@ import

  1   2   >