Re: SolrZKClient changed interface

2012-11-12 Thread Trym R. Møller 

Can anyone verify that the jira has been created sensible?
Thanks in advance.

https://issues.apache.org/jira/browse/SOLR-4066

Best regards Trym

Den 10-11-2012 00:54, Mark Miller skrev:

Please file a JIRA issue for this change.

- Mark

On Nov 9, 2012, at 8:41 AM, Trym R. Møller t...@sigmat.dk wrote:


Hi

The constructor of SolrZKClient has changed, I expect to ensure clean up of 
resources. The strategy is as follows:
connManager = new ConnectionManager(...)
try {
...
} catch (Throwable e) {
  connManager.close();
  throw new RuntimeException();
}
try {
  connManager.waitForConnected(clientConnectTimeout);
} catch (Throwable e) {
  connManager.close();
  throw new RuntimeException();
}

This results in a different exception (RuntimeException) returned from the 
constructor as earlier (nice exceptions as UnknownHostException, 
TimeoutException).

Can this be changed so we keep the old nice exceptions e.g. as follows 
(requiring the constructor to declare these) or at least include them as cause 
in the RuntimeException?

boolean closeBecauseOfException = true;
try {
...
   connManager.waitForConnected(clientConnectTimeout);
   closeBecauseOfException = false
} finally {
if (closeBecauseOfException) {
connManager.close();
}
}

Any comments appreciated.

Best regards Trym

http://svn.apache.org/repos/asf/lucene/dev/branches/lucene_solr_4_0/solr/solrj/src/java/org/apache/solr/common/cloud/SolrZkClient.java

SolrZKClient changed interface

2012-11-09 Thread Trym R. Møller 

Hi

The constructor of SolrZKClient has changed, I expect to ensure clean up 
of resources. The strategy is as follows:

connManager = new ConnectionManager(...)
try {
...
} catch (Throwable e) {
  connManager.close();
  throw new RuntimeException();
}
try {
  connManager.waitForConnected(clientConnectTimeout);
} catch (Throwable e) {
  connManager.close();
  throw new RuntimeException();
}

This results in a different exception (RuntimeException) returned from 
the constructor as earlier (nice exceptions as UnknownHostException, 
TimeoutException).


Can this be changed so we keep the old nice exceptions e.g. as follows 
(requiring the constructor to declare these) or at least include them as 
cause in the RuntimeException?


boolean closeBecauseOfException = true;
try {
...
   connManager.waitForConnected(clientConnectTimeout);
   closeBecauseOfException = false
} finally {
if (closeBecauseOfException) {
connManager.close();
}
}

Any comments appreciated.

Best regards Trym

http://svn.apache.org/repos/asf/lucene/dev/branches/lucene_solr_4_0/solr/solrj/src/java/org/apache/solr/common/cloud/SolrZkClient.java

Re: SolrZKClient changed interface

2012-11-09 Thread Per Steffensen 

Hi Trym

I believe one of the reasons that they started throwing 
RuntimeExceptions insted of UnknownHostException, TimeoutException etc 
is that the method signature has changed to not have a throws-part. 
They probably do not want do deal with those checked exceptions. Im not 
sure I completely understand your suggestion, but if they want to keep 
the method signature this way, there is not way to get checked 
excpetions (like UnknownHostException and TimeoutException) out of this 
method - therefore best solution is at least to include the actual 
exceptions as cause on the RuntimeExceptions. Or maybe it is just an 
accident that the checked excpetions was removed from the method 
signature, and then of course you can add them again at make sure the 
actual exceptions are propagated correctly.


Regards, Per Steffensen

Trym R. Møller skrev:

Hi

The constructor of SolrZKClient has changed, I expect to ensure clean 
up of resources. The strategy is as follows:

connManager = new ConnectionManager(...)
try {
...
} catch (Throwable e) {
  connManager.close();
  throw new RuntimeException();
}
try {
  connManager.waitForConnected(clientConnectTimeout);
} catch (Throwable e) {
  connManager.close();
  throw new RuntimeException();
}

This results in a different exception (RuntimeException) returned from 
the constructor as earlier (nice exceptions as UnknownHostException, 
TimeoutException).


Can this be changed so we keep the old nice exceptions e.g. as follows 
(requiring the constructor to declare these) or at least include them 
as cause in the RuntimeException?


boolean closeBecauseOfException = true;
try {
...
   connManager.waitForConnected(clientConnectTimeout);
   closeBecauseOfException = false
} finally {
if (closeBecauseOfException) {
connManager.close();
}
}

Any comments appreciated.

Best regards Trym

http://svn.apache.org/repos/asf/lucene/dev/branches/lucene_solr_4_0/solr/solrj/src/java/org/apache/solr/common/cloud/SolrZkClient.java

Re: SolrZKClient changed interface

2012-11-09 Thread Mark Miller 
Please file a JIRA issue for this change.

- Mark

On Nov 9, 2012, at 8:41 AM, Trym R. Møller t...@sigmat.dk wrote:

 Hi
 
 The constructor of SolrZKClient has changed, I expect to ensure clean up of 
 resources. The strategy is as follows:
 connManager = new ConnectionManager(...)
 try {
...
 } catch (Throwable e) {
  connManager.close();
  throw new RuntimeException();
 }
 try {
  connManager.waitForConnected(clientConnectTimeout);
 } catch (Throwable e) {
  connManager.close();
  throw new RuntimeException();
 }
 
 This results in a different exception (RuntimeException) returned from the 
 constructor as earlier (nice exceptions as UnknownHostException, 
 TimeoutException).
 
 Can this be changed so we keep the old nice exceptions e.g. as follows 
 (requiring the constructor to declare these) or at least include them as 
 cause in the RuntimeException?
 
 boolean closeBecauseOfException = true;
 try {
...
   connManager.waitForConnected(clientConnectTimeout);
   closeBecauseOfException = false
 } finally {
if (closeBecauseOfException) {
connManager.close();
}
 }
 
 Any comments appreciated.
 
 Best regards Trym
 
 http://svn.apache.org/repos/asf/lucene/dev/branches/lucene_solr_4_0/solr/solrj/src/java/org/apache/solr/common/cloud/SolrZkClient.java