Hi, Shay --
I agree on the documentation side as a minimum for the exceptions, although even that is a bit painful from a developer's perspective. (The compiler can do the work if the exceptions are declared.) It would be useful to be able to raise an exception (perhaps as a matter of policy, perhaps as the default behavior) if there are any shard failures or failures to acknowledge.
For the (purported) builder API issues, I went back over the code, and it looks like the only null guarding that was needed was for the aliases for an index, and the possibly null return value there is fair to expect of the developer because it's a get on a map.
-- Paul
On Aug 31, 2011, at 2:44 PM, Shay Banon wrote:
On Thu, Sep 1, 2011 at 12:35 AM, Paul Brown prb@mult.ifario.us wrote:
On Aug 31, 2011, at 2:25 PM, Shay Banon wrote:
Two important improvements just pushed to master (will be part of 0.18) that are there to correct confusing behavior with the Java API:
-
There is no more thread based caching of XContentBuilder, some have been bitten by it, and its not worth the trouble it created. Issue: Java API: XContentFactory creation of XContentBuilder to always be "safe" · Issue #1291 · elastic/elasticsearch · GitHub.
-
When working with ActionFuture (getting one from execute on the relevant prepareXXX methods), actionGet will now throw the unwrapped / action exception, and not a possibly wrapped one. Issue: Java API: ActionFuture#actionGet to automatically unwrap failures · Issue #1292 · elastic/elasticsearch · GitHub.
Speaking as a user of the Java API, #2 is a very welcome change, and it would be great if the specific exceptions that can be thrown (versus the general ElasticSearchException) are declared on the methods. The convenience method to determine whether the root cause is a particular type is useful, but it still has sent me reading through the Elasticsearch code to find out what exceptions may be thrown under which circumstances.
This is more of a documentation question, you can't list all the possible exception for different APIs on the same "ActionFuture".
In terms of other API clean-up suitable for a version jump, there are some other situations where some builder-style methods can return null (I'm thinking specifically of accessing admin information about indexes), and it would be great to clean that up as well. (My main objection to builder-style APIs is not to the builder-style itself but rather that people often break the contract of non-null return values…)
I did not follow you, maybe a concrete example?
-- Paul