On Thu, Aug 9, 2012 at 4:51 PM, Robbie Gemmell <[email protected]> wrote: > On 9 August 2012 21:31, Rajith Attapattu <[email protected]> wrote: > >> On Thu, Aug 9, 2012 at 4:13 PM, Robbie Gemmell <[email protected]> >> wrote: >> > No, "connecttimeout" being unused isn't intended, but it appears to have >> > been that way since 0.6 went out, see: >> > https://issues.apache.org/jira/browse/QPID-4051. >> >> So perhaps a bug ? perhaps we should fix this for the next release ? >> >> > Defintiely a bug, I actually meant to fix it for this release but you can > kinda tell from the comments on the JIRA that I went on holiday (<gasp>) > and forgot about it :P
I saw your comments on the JIRA :) Now that we are all working on the same code base we tend to catch things more often. > >> > "qpid.failover_method_timeout" is not intended to be used instead of it >> but >> > is instead a very recently introduced system property that can be used to >> > control a previously hard coded value in an entirely different area of >> > code. As someone once said in a commit message "hard coding makes coding >> > hard". This is a good example of one of those 'we dont really expect/want >> > people to use and so shouldnt bother to confuse them by documenting it' >> > system properties I just mentioned in my other email. >> >> However due to connecttimeout not working (or not used) it appears >> this property was somewhat useful to an end user. >> But I agree that if this isn't generally useful then better not to >> document it. >> Instead maybe we should fix connecttimeout. >> >> > Given that connecttimeout has been broken for 3 years before being noticed > I wouldn't bother to document this one (which actually does something > different, not normally all that useful to a user, particularly if they > need to timeout a connect() operation) but instead fix connecttimeout which > is documented and only ever mention this if its actually necessary. > > We have put a connect() timeout in for 0.16 based on a user patch > (QPID-4047), albeit it isnt configurable due to QPID-4051 but in this case > thats still better than no timeout in the previous releases. > > >> P.S I guess we will have to do some of these minor fixes until we >> provide a fully working new JMS client. >> The connect timeout seems a useful feature when used with the >> singlebroker situation. >> > > Agreed, a connect() timeout could be useful with any connection situation > so we should definitely fix it. Indeed! Thanks Robbie! Rajith > >> Rajith >> >> > Robbie >> >> >> > >> > On 9 August 2012 16:10, Rajith Attapattu <[email protected]> wrote: >> > >> >> While looking up the code to answer a question from a user, I noticed >> >> that we don't use "connecttimeout" any where. >> >> This property is retrieved using the "getTimeout()" method in >> >> AMQBrokerDetails.java >> >> >> >> However the getTimeout() method (or the property) is not used any >> >> where else in the code. >> >> Instead it appears we use "qpid.failover_method_timeout" with a >> >> default of 120000 >> >> (We should also document these system properties in the docs) >> >> >> >> Is this behaviour correct and as expected ? >> >> Could anybody who is familiar with this change/feature shed some light >> on >> >> this? >> >> >> >> Regards, >> >> >> >> Rajith >> >> >> >> --------------------------------------------------------------------- >> >> To unsubscribe, e-mail: [email protected] >> >> For additional commands, e-mail: [email protected] >> >> >> >> >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: [email protected] >> For additional commands, e-mail: [email protected] >> >> --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
