Re: Review Request: QPID-3345: restore/add ability to use sys props to select the NetworkTransport used to make/accept connections
On 2011-07-13 04:04:38, rajith attapattu wrote: /trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/network/Transport.java, line 30 https://reviews.apache.org/r/1087/diff/1/?file=22375#file22375line30 Looking at the Transport class, I see that transports are choosen based on the AMQP protocol version. While it is true that this can be easily overridden using qpid.transport system property, it would have been nice if we had the transport implementations independent of the protocol versions. Perhaps this is the case, and the map is just there to specify the default (or preferred) transport for each version? Is this approach was taken due to the 0-8,0-9 version code paths are heavily tied to MINA (I haven't really looked at the code in this area for a long time) ? No, for the client, user can either override all transports (via -Dqpid.transport) or by protocol version (via -Dqpid.transport.v0_10 etc). The objective of this Jira was merely to reintroduce plugability of the existing transports. The next step will be the testing of the IoNetworkTransport implementation against 0-8..0-9-1 so that it can become the default for all protocols. This will take us the next step towards making the use Mina on the client optional. This is a reasonably complex area, so I think an incremental approach makes sense. - Keith --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1087/#review1041 --- On 2011-07-12 09:38:50, Keith Wall wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1087/ --- (Updated 2011-07-12 09:38:50) Review request for qpid. Summary --- QPID-3345: transport implementations for client/broker sides controllable via new System Properties and Reflection. This addresses bug QPID-3345. https://issues.apache.org/jira/browse/QPID-3345 Diffs - /trunk/qpid/java/common/src/test/java/org/apache/qpid/transport/network/TransportTest.java PRE-CREATION /trunk/qpid/java/common/src/test/java/org/apache/qpid/test/utils/QpidTestCase.java 1145481 /trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/network/security/SecurityLayer.java 1145481 /trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/network/Transport.java 1145481 /trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/network/NetworkTransport.java 1145481 /trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/Broker.java 1145481 /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_8_0.java 1145481 /trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Connection.java 1145481 /trunk/qpid/java/systests/src/main/java/org/apache/qpid/test/utils/QpidBrokerTestCase.java 1145481 Diff: https://reviews.apache.org/r/1087/diff Testing --- Additional unit test and exercised by existing system test suite (0-9-1/0-10 code paths) Thanks, Keith
Re: Review Request: QPID-3345: restore/add ability to use sys props to select the NetworkTransport used to make/accept connections
On 2011-07-13 04:04:38, rajith attapattu wrote: /trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/network/Transport.java, line 30 https://reviews.apache.org/r/1087/diff/1/?file=22375#file22375line30 Looking at the Transport class, I see that transports are choosen based on the AMQP protocol version. While it is true that this can be easily overridden using qpid.transport system property, it would have been nice if we had the transport implementations independent of the protocol versions. Perhaps this is the case, and the map is just there to specify the default (or preferred) transport for each version? Is this approach was taken due to the 0-8,0-9 version code paths are heavily tied to MINA (I haven't really looked at the code in this area for a long time) ? Keith Wall wrote: No, for the client, user can either override all transports (via -Dqpid.transport) or by protocol version (via -Dqpid.transport.v0_10 etc). The objective of this Jira was merely to reintroduce plugability of the existing transports. The next step will be the testing of the IoNetworkTransport implementation against 0-8..0-9-1 so that it can become the default for all protocols. This will take us the next step towards making the use Mina on the client optional. This is a reasonably complex area, so I think an incremental approach makes sense. Gotcha. Thanks for the answers. - rajith --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1087/#review1041 --- On 2011-07-12 09:38:50, Keith Wall wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1087/ --- (Updated 2011-07-12 09:38:50) Review request for qpid. Summary --- QPID-3345: transport implementations for client/broker sides controllable via new System Properties and Reflection. This addresses bug QPID-3345. https://issues.apache.org/jira/browse/QPID-3345 Diffs - /trunk/qpid/java/common/src/test/java/org/apache/qpid/transport/network/TransportTest.java PRE-CREATION /trunk/qpid/java/common/src/test/java/org/apache/qpid/test/utils/QpidTestCase.java 1145481 /trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/network/security/SecurityLayer.java 1145481 /trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/network/Transport.java 1145481 /trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/network/NetworkTransport.java 1145481 /trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/Broker.java 1145481 /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_8_0.java 1145481 /trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Connection.java 1145481 /trunk/qpid/java/systests/src/main/java/org/apache/qpid/test/utils/QpidBrokerTestCase.java 1145481 Diff: https://reviews.apache.org/r/1087/diff Testing --- Additional unit test and exercised by existing system test suite (0-9-1/0-10 code paths) Thanks, Keith
Re: Review Request: QPID-3345: restore/add ability to use sys props to select the NetworkTransport used to make/accept connections
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1087/#review1041 --- /trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/network/Transport.java https://reviews.apache.org/r/1087/#comment2117 Looking at the Transport class, I see that transports are choosen based on the AMQP protocol version. While it is true that this can be easily overridden using qpid.transport system property, it would have been nice if we had the transport implementations independent of the protocol versions. Perhaps this is the case, and the map is just there to specify the default (or preferred) transport for each version? Is this approach was taken due to the 0-8,0-9 version code paths are heavily tied to MINA (I haven't really looked at the code in this area for a long time) ? - rajith On 2011-07-12 09:38:50, Keith Wall wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1087/ --- (Updated 2011-07-12 09:38:50) Review request for qpid. Summary --- QPID-3345: transport implementations for client/broker sides controllable via new System Properties and Reflection. This addresses bug QPID-3345. https://issues.apache.org/jira/browse/QPID-3345 Diffs - /trunk/qpid/java/common/src/test/java/org/apache/qpid/transport/network/TransportTest.java PRE-CREATION /trunk/qpid/java/common/src/test/java/org/apache/qpid/test/utils/QpidTestCase.java 1145481 /trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/network/security/SecurityLayer.java 1145481 /trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/network/Transport.java 1145481 /trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/network/NetworkTransport.java 1145481 /trunk/qpid/java/broker/src/main/java/org/apache/qpid/server/Broker.java 1145481 /trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_8_0.java 1145481 /trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Connection.java 1145481 /trunk/qpid/java/systests/src/main/java/org/apache/qpid/test/utils/QpidBrokerTestCase.java 1145481 Diff: https://reviews.apache.org/r/1087/diff Testing --- Additional unit test and exercised by existing system test suite (0-9-1/0-10 code paths) Thanks, Keith
