You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@curator.apache.org by Mike Drob <ma...@cloudera.com> on 2014/08/01 21:31:20 UTC
Exception throwing
I'd like to revisit the discussion around always throwing Exception in the
API. There were two JIRA issues - CURATOR-135 and CURATOR-29 - that touch
on this subject, but I think there is a good conversation to be had.
I understand the suggestions that if an exception is thrown, we are in a
bad state, regardless of the type of exception. However, throwing Exception
comes with some unfortunate Java baggage...
By declaring thrown Exception, we force consumers to also catch
RuntimeExceptions instead of letting them propagate as they normally would.
In some cases, the calling code may need to attempt roll-back semantics, or
retry outside of what Curator provides, or something else that we haven't
thought of.
I'd like to propose replacing much of the thrown Exception methods with
CuratorException. This will still carry the connotation that it doesn't
matter what kind of exception we encounter, they're all bad. It will also
be backwards compatible with the previous API, since users will still be
able to catch Exception in their calling code. And it has the advantage of
separating checked exceptions from unchecked ones, so that users don't
unintentionally catch something unrelated.
Thoughts?
I tried looking for more details behind the design decision to always throw
Exception, but wasn't able to find them. If they're already documented, I'd
love to be pointed at the wiki or site, or a mailing list thread will do in
a pinch.
Thanks,
Mike
Re: Exception throwing
Posted by Jordan Zimmerman <jo...@jordanzimmerman.com>.
I already have a placeholder version 3.0 in Jira:
https://issues.apache.org/jira/browse/CURATOR/fixforversion/12326664/?selectedTab=com.atlassian.jira.jira-projects-plugin:version-summary-panel
We could start putting together a Curator 3.0 wish list and iterate.
-JZ
From: Mike Drob <ma...@cloudera.com>
Reply: dev@curator.apache.org <de...@curator.apache.org>>
Date: August 4, 2014 at 9:29:14 AM
To: Cameron McKenzie <mc...@gmail.com>>
Cc: dev@curator.apache.org <de...@curator.apache.org>>, vines@apache.org <vi...@apache.org>>
Subject: Re: Exception throwing
I can see how it may be unappealing modifying everything in the library to
accommodate this. And I think I figured out why this matters (before I was
suggesting it from mostly intuition).
Jordan, you posit that Curator assumes familiarity with ZooKeeper. In some
cases, that's warranted, but in others, I'm not so sure. I understand the
basic ZooKeeper semantics - you can put and get data, it provides quorum
guarantees, data is stored hierarchically, etc... However, I don't know the
ins and outs of the ZooKeeper API. I know that KeeperException is a thing,
but I couldn't tell you which methods throw it, why they throw it, and what
all the (enum? int? string?) codes are. Dealing directly with ZooKeeper is
thorny!
Enter Curator! It gives me so much for free - retry policies, sanity
checking, a testing server for unit tests, it's great. Until I run into a
KeeperException. That's a very jarring experience because I thought that
Curator protected me from such nonsense. And it never advertised that
KeeperException was a possibility! My only indication was the 'throws
Exception' clause on most methods, which I have been taught by the library
to dutifully ignore it and just fail fast. Had I seen a 'throws
KeeperException' declaration, I might have thought about what that means,
but that information was not available to me.
Yes, it is unfortunate that ZooKeeper uses exceptions to return result
codes. That doesn't mean that we have to do the same. I would love to see
CreateBuilder implement BackgroundPathAndBytesable<Boolean> instead of
String and for it to eat the KeeperException. Or maybe return a Stat. Just
protect me from that exception.
Mike
On Sat, Aug 2, 2014 at 5:47 PM, Cameron McKenzie <mc...@gmail.com>
wrote:
> I don't have a super strong view either way. If we were designing from
> scratch I'd go with checked exceptions, just because that's how I prefer to
> code.
>
> Having said that, I'm not convinced that modifying the whole code base at
> this point to add a CuratorException is worth the effort.
>
>
> On Sat, Aug 2, 2014 at 8:09 AM, Jordan Zimmerman <
> jordan@jordanzimmerman.com> wrote:
>
>> Curator assumes familiarity with ZooKeeper. Of course, the docs should be
>> improved where needed. Curator doesn’t hide any KeeperExceptions except
>> where it advertises that it handles them - e.g. creatingParentsIfNeeded().
>> The fact that ZooKeeper uses exceptions to return result codes for correct
>> behavior is the problem here.
>>
>> That said, I wrote nearly all of the Curator recipes and have never had
>> an occasion where Curator’s use of Exception was a problem.
>>
>> -JZ
>>
>> From: Mike Drob <ma...@cloudera.com>
>> Reply: dev@curator.apache.org <de...@curator.apache.org>>
>> Date: August 1, 2014 at 5:05:04 PM
>> To: vines@apache.org <vi...@apache.org>>
>> Cc: dev@curator.apache.org <de...@curator.apache.org>>
>> Subject: Re: Exception throwing
>>
>> The set with version is basically a compareAndSet. java.util chooses to
>> implement this also with a 'return false' value for when some other thread
>> got there first. I'll have to go back and look at the Curator API and see
>> how these failures are currently communicated.
>>
>>
>> On Fri, Aug 1, 2014 at 4:08 PM, John Vines <vi...@apache.org> wrote:
>>
>> > There's KeeperException.BADVERSION, which is when you
>> > setData().withVersion and the version in ZK changed vs. the version seen
>> > prior. That one is pretty critical to support, IMO. The other cases
>> around
>> > node existance can definitely be handled by the user, but given the
>> > possibilities for races in distributed systems you still can't be
>> certain.
>> > But there could be user cases where they want to create a node and not
>> fail
>> > if it exists or have a delete pass if the node was already deleted by
>> > something else (not sure if a flag was added for that case, I vaguely
>> > recall a discussion).
>> >
>> >
>> > On Fri, Aug 1, 2014 at 5:04 PM, Mike Drob <ma...@cloudera.com> wrote:
>> >
>> >> John, thanks for your input. So some of this is improper use of the
>> API,
>> >> right? If you are attempting to create a node and it already exists,
>> then
>> >> that can be an exceptional case. If you just want to make sure that a
>> node
>> >> exists, regardless of who created it, then that's a case for a
>> different
>> >> API. Asserting that you created the node could be important - see the
>> >> distinction in ConcurrentMap between put() and putIfAbsent(). Then
>> again,
>> >> failing to create the node because it already exists might not need to
>> be
>> >> an exceptional case and simply warrants a 'return false' on the
>> method? Do
>> >> the other cases you mentioned have similar analogues? Maybe the end
>> result
>> >> of what comes out of this is better docs.
>> >>
>> >> Mike
>> >>
>> >>
>> >> On Fri, Aug 1, 2014 at 3:39 PM, John Vines <vi...@apache.org> wrote:
>> >>
>> >>> It's not a matter of it being a bug, it's a matter of usability.
>> Because
>> >>> every single method just throws Exception it gives me, as a user,
>> >>> absolutely zero inclination at writing time to figure out what sort of
>> >>> failures can happen. And the complete lack of javadocs compound this
>> >>> issue.
>> >>> This has been my biggest issue with Curator.
>> >>>
>> >>> Yes, there are some unrecoverable errors. But not all of them are,
>> such
>> >>> as
>> >>> a subset of the KeeperExceptions around node state, security, and
>> >>> versions.
>> >>> I could be sold on a split, where those type of items are exposed and
>> >>> then
>> >>> the critical ones you keep mentioning are Runtime. But as much as I
>> >>> dislike
>> >>> generic Exceptions for everything, forcing users to catch
>> >>> RuntimeExceptions
>> >>> to do proper exception handling for known and well defined exceptions
>> is
>> >>> an
>> >>> awful practice to put people though.
>> >>>
>> >>>
>> >>> On Fri, Aug 1, 2014 at 3:49 PM, Jordan Zimmerman <
>> >>> jordan@jordanzimmerman.com
>> >>> > wrote:
>> >>>
>> >>> > -1 (binding)
>> >>> >
>> >>> > If I could go back I’d remove all checked exceptions entirely. I
>> don’t
>> >>> > think there’s an objective answer here - it comes down to personal
>> >>> > preference, etc. I don’t see much value in touching nearly every
>> file
>> >>> in
>> >>> > the library in order to do this. We’ve had maybe 2 or 3 requests in
>> the
>> >>> > many years that Curator has exists. This suggests that the
>> overwhelming
>> >>> > majority accept the current exception semantics. If you can point
>> to an
>> >>> > actual bug that this causes that would be helpful.
>> >>> >
>> >>> > -Jordan
>> >>> >
>> >>> > From: Mike Drob <ma...@cloudera.com>
>> >>> > Reply: dev@curator.apache.org <de...@curator.apache.org>>
>> >>> > Date: August 1, 2014 at 2:32:07 PM
>> >>> > To: dev@curator.apache.org <de...@curator.apache.org>>
>> >>> > Subject: Exception throwing
>> >>> >
>> >>> > I'd like to revisit the discussion around always throwing Exception
>> in
>> >>> the
>> >>> > API. There were two JIRA issues - CURATOR-135 and CURATOR-29 - that
>> >>> touch
>> >>> > on this subject, but I think there is a good conversation to be had.
>> >>> >
>> >>> > I understand the suggestions that if an exception is thrown, we are
>> in
>> >>> a
>> >>> > bad state, regardless of the type of exception. However, throwing
>> >>> Exception
>> >>> > comes with some unfortunate Java baggage...
>> >>> >
>> >>> > By declaring thrown Exception, we force consumers to also catch
>> >>> > RuntimeExceptions instead of letting them propagate as they normally
>> >>> would.
>> >>> > In some cases, the calling code may need to attempt roll-back
>> >>> semantics, or
>> >>> > retry outside of what Curator provides, or something else that we
>> >>> haven't
>> >>> > thought of.
>> >>> >
>> >>> > I'd like to propose replacing much of the thrown Exception methods
>> with
>> >>> > CuratorException. This will still carry the connotation that it
>> doesn't
>> >>> > matter what kind of exception we encounter, they're all bad. It will
>> >>> also
>> >>> > be backwards compatible with the previous API, since users will
>> still
>> >>> be
>> >>> > able to catch Exception in their calling code. And it has the
>> >>> advantage of
>> >>> > separating checked exceptions from unchecked ones, so that users
>> don't
>> >>> > unintentionally catch something unrelated.
>> >>> >
>> >>> > Thoughts?
>> >>> >
>> >>> > I tried looking for more details behind the design decision to
>> always
>> >>> throw
>> >>> > Exception, but wasn't able to find them. If they're already
>> >>> documented, I'd
>> >>> > love to be pointed at the wiki or site, or a mailing list thread
>> will
>> >>> do in
>> >>> > a pinch.
>> >>> >
>> >>> > Thanks,
>> >>> > Mike
>> >>> >
>> >>>
>> >>
>> >>
>> >
>>
>
>
Re: Exception throwing
Posted by Mike Drob <ma...@cloudera.com>.
This is good. How do we get there?
On Mon, Aug 4, 2014 at 9:36 AM, Jordan Zimmerman <jordan@jordanzimmerman.com
> wrote:
> Just
> protect me from that exception.
>
> I agree with this.
> I’d like to see Curator move to unchecked exceptions everywhere. For
> “recoverable exceptions”, I’d like to see something like this (BTW - I SO
> wish I did this initially): have the various forPath() methods return a
> class (rather than byte[], etc.) like:
>
> public interface CuratorResult
> {
> public byte[] getBytes();
>
> public CuratorResultType getResultType();
> }
>
> public enum CuratorResultType
> {
> SUCCESS,
> NO_NODE,
> NODE_EXISTS,
> BAD_VERSION
> … etc
> }
>
> To me, this is FAR superior than the recoverable exceptions. It would also
> make the APIs a lot easier to use. You could do:
>
> CuratorResult result = client.create().inBackground().forPath(myPath);
> if ( result.getResultType() == SUCCESS )
> {
> // etc.
> }
> else
> {
> // etc.
> }
>
> From: Mike Drob <ma...@cloudera.com> <ma...@cloudera.com>
> Reply: dev@curator.apache.org <de...@curator.apache.org>>
> <de...@curator.apache.org>
> Date: August 4, 2014 at 9:29:14 AM
> To: Cameron McKenzie <mc...@gmail.com>> <mc...@gmail.com>
> Cc: dev@curator.apache.org <de...@curator.apache.org>>
> <de...@curator.apache.org>, vines@apache.org <vi...@apache.org>>
> <vi...@apache.org>
> Subject: Re: Exception throwing
>
> I can see how it may be unappealing modifying everything in the library to
> accommodate this. And I think I figured out why this matters (before I was
> suggesting it from mostly intuition).
>
> Jordan, you posit that Curator assumes familiarity with ZooKeeper. In some
> cases, that's warranted, but in others, I'm not so sure. I understand the
> basic ZooKeeper semantics - you can put and get data, it provides quorum
> guarantees, data is stored hierarchically, etc... However, I don't know
> the
> ins and outs of the ZooKeeper API. I know that KeeperException is a thing,
> but I couldn't tell you which methods throw it, why they throw it, and
> what
> all the (enum? int? string?) codes are. Dealing directly with ZooKeeper is
> thorny!
>
> Enter Curator! It gives me so much for free - retry policies, sanity
> checking, a testing server for unit tests, it's great. Until I run into a
> KeeperException. That's a very jarring experience because I thought that
> Curator protected me from such nonsense. And it never advertised that
> KeeperException was a possibility! My only indication was the 'throws
> Exception' clause on most methods, which I have been taught by the library
> to dutifully ignore it and just fail fast. Had I seen a 'throws
> KeeperException' declaration, I might have thought about what that means,
> but that information was not available to me.
>
> Yes, it is unfortunate that ZooKeeper uses exceptions to return result
> codes. That doesn't mean that we have to do the same. I would love to see
> CreateBuilder implement BackgroundPathAndBytesable<Boolean> instead of
> String and for it to eat the KeeperException. Or maybe return a Stat. Just
> protect me from that exception.
>
> Mike
>
>
>
> On Sat, Aug 2, 2014 at 5:47 PM, Cameron McKenzie <mc...@gmail.com>
> wrote:
>
> > I don't have a super strong view either way. If we were designing from
> > scratch I'd go with checked exceptions, just because that's how I prefer
> to
> > code.
> >
> > Having said that, I'm not convinced that modifying the whole code base
> at
> > this point to add a CuratorException is worth the effort.
> >
> >
> > On Sat, Aug 2, 2014 at 8:09 AM, Jordan Zimmerman <
> > jordan@jordanzimmerman.com> wrote:
> >
> >> Curator assumes familiarity with ZooKeeper. Of course, the docs should
> be
> >> improved where needed. Curator doesn’t hide any KeeperExceptions except
> >> where it advertises that it handles them - e.g.
> creatingParentsIfNeeded().
> >> The fact that ZooKeeper uses exceptions to return result codes for
> correct
> >> behavior is the problem here.
> >>
> >> That said, I wrote nearly all of the Curator recipes and have never had
> >> an occasion where Curator’s use of Exception was a problem.
> >>
> >> -JZ
> >>
> >> From: Mike Drob <ma...@cloudera.com>
> >> Reply: dev@curator.apache.org <de...@curator.apache.org>>
> >> Date: August 1, 2014 at 5:05:04 PM
> >> To: vines@apache.org <vi...@apache.org>>
> >> Cc: dev@curator.apache.org <de...@curator.apache.org>>
> >> Subject: Re: Exception throwing
> >>
> >> The set with version is basically a compareAndSet. java.util chooses to
> >> implement this also with a 'return false' value for when some other
> thread
> >> got there first. I'll have to go back and look at the Curator API and
> see
> >> how these failures are currently communicated.
> >>
> >>
> >> On Fri, Aug 1, 2014 at 4:08 PM, John Vines <vi...@apache.org> wrote:
> >>
> >> > There's KeeperException.BADVERSION, which is when you
> >> > setData().withVersion and the version in ZK changed vs. the version
> seen
> >> > prior. That one is pretty critical to support, IMO. The other cases
> >> around
> >> > node existance can definitely be handled by the user, but given the
> >> > possibilities for races in distributed systems you still can't be
> >> certain.
> >> > But there could be user cases where they want to create a node and
> not
> >> fail
> >> > if it exists or have a delete pass if the node was already deleted by
> >> > something else (not sure if a flag was added for that case, I vaguely
> >> > recall a discussion).
> >> >
> >> >
> >> > On Fri, Aug 1, 2014 at 5:04 PM, Mike Drob <ma...@cloudera.com>
> wrote:
> >> >
> >> >> John, thanks for your input. So some of this is improper use of the
> >> API,
> >> >> right? If you are attempting to create a node and it already exists,
> >> then
> >> >> that can be an exceptional case. If you just want to make sure that
> a
> >> node
> >> >> exists, regardless of who created it, then that's a case for a
> >> different
> >> >> API. Asserting that you created the node could be important - see
> the
> >> >> distinction in ConcurrentMap between put() and putIfAbsent(). Then
> >> again,
> >> >> failing to create the node because it already exists might not need
> to
> >> be
> >> >> an exceptional case and simply warrants a 'return false' on the
> >> method? Do
> >> >> the other cases you mentioned have similar analogues? Maybe the end
> >> result
> >> >> of what comes out of this is better docs.
> >> >>
> >> >> Mike
> >> >>
> >> >>
> >> >> On Fri, Aug 1, 2014 at 3:39 PM, John Vines <vi...@apache.org>
> wrote:
> >> >>
> >> >>> It's not a matter of it being a bug, it's a matter of usability.
> >> Because
> >> >>> every single method just throws Exception it gives me, as a user,
> >> >>> absolutely zero inclination at writing time to figure out what sort
> of
> >> >>> failures can happen. And the complete lack of javadocs compound
> this
> >> >>> issue.
> >> >>> This has been my biggest issue with Curator.
> >> >>>
> >> >>> Yes, there are some unrecoverable errors. But not all of them are,
> >> such
> >> >>> as
> >> >>> a subset of the KeeperExceptions around node state, security, and
> >> >>> versions.
> >> >>> I could be sold on a split, where those type of items are exposed
> and
> >> >>> then
> >> >>> the critical ones you keep mentioning are Runtime. But as much as I
> >> >>> dislike
> >> >>> generic Exceptions for everything, forcing users to catch
> >> >>> RuntimeExceptions
> >> >>> to do proper exception handling for known and well defined
> exceptions
> >> is
> >> >>> an
> >> >>> awful practice to put people though.
> >> >>>
> >> >>>
> >> >>> On Fri, Aug 1, 2014 at 3:49 PM, Jordan Zimmerman <
> >> >>> jordan@jordanzimmerman.com
> >> >>> > wrote:
> >> >>>
> >> >>> > -1 (binding)
> >> >>> >
> >> >>> > If I could go back I’d remove all checked exceptions entirely. I
> >> don’t
> >> >>> > think there’s an objective answer here - it comes down to
> personal
> >> >>> > preference, etc. I don’t see much value in touching nearly every
> >> file
> >> >>> in
> >> >>> > the library in order to do this. We’ve had maybe 2 or 3 requests
> in
> >> the
> >> >>> > many years that Curator has exists. This suggests that the
> >> overwhelming
> >> >>> > majority accept the current exception semantics. If you can point
> >> to an
> >> >>> > actual bug that this causes that would be helpful.
> >> >>> >
> >> >>> > -Jordan
> >> >>> >
> >> >>> > From: Mike Drob <ma...@cloudera.com>
> >> >>> > Reply: dev@curator.apache.org <de...@curator.apache.org>>
> >> >>> > Date: August 1, 2014 at 2:32:07 PM
> >> >>> > To: dev@curator.apache.org <de...@curator.apache.org>>
> >> >>> > Subject: Exception throwing
> >> >>> >
> >> >>> > I'd like to revisit the discussion around always throwing
> Exception
> >> in
> >> >>> the
> >> >>> > API. There were two JIRA issues - CURATOR-135 and CURATOR-29 -
> that
> >> >>> touch
> >> >>> > on this subject, but I think there is a good conversation to be
> had.
> >> >>> >
> >> >>> > I understand the suggestions that if an exception is thrown, we
> are
> >> in
> >> >>> a
> >> >>> > bad state, regardless of the type of exception. However, throwing
> >> >>> Exception
> >> >>> > comes with some unfortunate Java baggage...
> >> >>> >
> >> >>> > By declaring thrown Exception, we force consumers to also catch
> >> >>> > RuntimeExceptions instead of letting them propagate as they
> normally
> >> >>> would.
> >> >>> > In some cases, the calling code may need to attempt roll-back
> >> >>> semantics, or
> >> >>> > retry outside of what Curator provides, or something else that we
> >> >>> haven't
> >> >>> > thought of.
> >> >>> >
> >> >>> > I'd like to propose replacing much of the thrown Exception
> methods
> >> with
> >> >>> > CuratorException. This will still carry the connotation that it
> >> doesn't
> >> >>> > matter what kind of exception we encounter, they're all bad. It
> will
> >> >>> also
> >> >>> > be backwards compatible with the previous API, since users will
> >> still
> >> >>> be
> >> >>> > able to catch Exception in their calling code. And it has the
> >> >>> advantage of
> >> >>> > separating checked exceptions from unchecked ones, so that users
> >> don't
> >> >>> > unintentionally catch something unrelated.
> >> >>> >
> >> >>> > Thoughts?
> >> >>> >
> >> >>> > I tried looking for more details behind the design decision to
> >> always
> >> >>> throw
> >> >>> > Exception, but wasn't able to find them. If they're already
> >> >>> documented, I'd
> >> >>> > love to be pointed at the wiki or site, or a mailing list thread
> >> will
> >> >>> do in
> >> >>> > a pinch.
> >> >>> >
> >> >>> > Thanks,
> >> >>> > Mike
> >> >>> >
> >> >>>
> >> >>
> >> >>
> >> >
> >>
> >
> >
>
>
Re: Exception throwing
Posted by Jordan Zimmerman <jo...@jordanzimmerman.com>.
Just
protect me from that exception.
I agree with this.
I’d like to see Curator move to unchecked exceptions everywhere. For “recoverable exceptions”, I’d like to see something like this (BTW - I SO wish I did this initially): have the various forPath() methods return a class (rather than byte[], etc.) like:
public interface CuratorResult
{
public byte[] getBytes();
public CuratorResultType getResultType();
}
public enum CuratorResultType
{
SUCCESS,
NO_NODE,
NODE_EXISTS,
BAD_VERSION
… etc
}
To me, this is FAR superior than the recoverable exceptions. It would also make the APIs a lot easier to use. You could do:
CuratorResult result = client.create().inBackground().forPath(myPath);
if ( result.getResultType() == SUCCESS )
{
// etc.
}
else
{
// etc.
}
From: Mike Drob <ma...@cloudera.com>
Reply: dev@curator.apache.org <de...@curator.apache.org>>
Date: August 4, 2014 at 9:29:14 AM
To: Cameron McKenzie <mc...@gmail.com>>
Cc: dev@curator.apache.org <de...@curator.apache.org>>, vines@apache.org <vi...@apache.org>>
Subject: Re: Exception throwing
I can see how it may be unappealing modifying everything in the library to
accommodate this. And I think I figured out why this matters (before I was
suggesting it from mostly intuition).
Jordan, you posit that Curator assumes familiarity with ZooKeeper. In some
cases, that's warranted, but in others, I'm not so sure. I understand the
basic ZooKeeper semantics - you can put and get data, it provides quorum
guarantees, data is stored hierarchically, etc... However, I don't know the
ins and outs of the ZooKeeper API. I know that KeeperException is a thing,
but I couldn't tell you which methods throw it, why they throw it, and what
all the (enum? int? string?) codes are. Dealing directly with ZooKeeper is
thorny!
Enter Curator! It gives me so much for free - retry policies, sanity
checking, a testing server for unit tests, it's great. Until I run into a
KeeperException. That's a very jarring experience because I thought that
Curator protected me from such nonsense. And it never advertised that
KeeperException was a possibility! My only indication was the 'throws
Exception' clause on most methods, which I have been taught by the library
to dutifully ignore it and just fail fast. Had I seen a 'throws
KeeperException' declaration, I might have thought about what that means,
but that information was not available to me.
Yes, it is unfortunate that ZooKeeper uses exceptions to return result
codes. That doesn't mean that we have to do the same. I would love to see
CreateBuilder implement BackgroundPathAndBytesable<Boolean> instead of
String and for it to eat the KeeperException. Or maybe return a Stat. Just
protect me from that exception.
Mike
On Sat, Aug 2, 2014 at 5:47 PM, Cameron McKenzie <mc...@gmail.com>
wrote:
> I don't have a super strong view either way. If we were designing from
> scratch I'd go with checked exceptions, just because that's how I prefer to
> code.
>
> Having said that, I'm not convinced that modifying the whole code base at
> this point to add a CuratorException is worth the effort.
>
>
> On Sat, Aug 2, 2014 at 8:09 AM, Jordan Zimmerman <
> jordan@jordanzimmerman.com> wrote:
>
>> Curator assumes familiarity with ZooKeeper. Of course, the docs should be
>> improved where needed. Curator doesn’t hide any KeeperExceptions except
>> where it advertises that it handles them - e.g. creatingParentsIfNeeded().
>> The fact that ZooKeeper uses exceptions to return result codes for correct
>> behavior is the problem here.
>>
>> That said, I wrote nearly all of the Curator recipes and have never had
>> an occasion where Curator’s use of Exception was a problem.
>>
>> -JZ
>>
>> From: Mike Drob <ma...@cloudera.com>
>> Reply: dev@curator.apache.org <de...@curator.apache.org>>
>> Date: August 1, 2014 at 5:05:04 PM
>> To: vines@apache.org <vi...@apache.org>>
>> Cc: dev@curator.apache.org <de...@curator.apache.org>>
>> Subject: Re: Exception throwing
>>
>> The set with version is basically a compareAndSet. java.util chooses to
>> implement this also with a 'return false' value for when some other thread
>> got there first. I'll have to go back and look at the Curator API and see
>> how these failures are currently communicated.
>>
>>
>> On Fri, Aug 1, 2014 at 4:08 PM, John Vines <vi...@apache.org> wrote:
>>
>> > There's KeeperException.BADVERSION, which is when you
>> > setData().withVersion and the version in ZK changed vs. the version seen
>> > prior. That one is pretty critical to support, IMO. The other cases
>> around
>> > node existance can definitely be handled by the user, but given the
>> > possibilities for races in distributed systems you still can't be
>> certain.
>> > But there could be user cases where they want to create a node and not
>> fail
>> > if it exists or have a delete pass if the node was already deleted by
>> > something else (not sure if a flag was added for that case, I vaguely
>> > recall a discussion).
>> >
>> >
>> > On Fri, Aug 1, 2014 at 5:04 PM, Mike Drob <ma...@cloudera.com> wrote:
>> >
>> >> John, thanks for your input. So some of this is improper use of the
>> API,
>> >> right? If you are attempting to create a node and it already exists,
>> then
>> >> that can be an exceptional case. If you just want to make sure that a
>> node
>> >> exists, regardless of who created it, then that's a case for a
>> different
>> >> API. Asserting that you created the node could be important - see the
>> >> distinction in ConcurrentMap between put() and putIfAbsent(). Then
>> again,
>> >> failing to create the node because it already exists might not need to
>> be
>> >> an exceptional case and simply warrants a 'return false' on the
>> method? Do
>> >> the other cases you mentioned have similar analogues? Maybe the end
>> result
>> >> of what comes out of this is better docs.
>> >>
>> >> Mike
>> >>
>> >>
>> >> On Fri, Aug 1, 2014 at 3:39 PM, John Vines <vi...@apache.org> wrote:
>> >>
>> >>> It's not a matter of it being a bug, it's a matter of usability.
>> Because
>> >>> every single method just throws Exception it gives me, as a user,
>> >>> absolutely zero inclination at writing time to figure out what sort of
>> >>> failures can happen. And the complete lack of javadocs compound this
>> >>> issue.
>> >>> This has been my biggest issue with Curator.
>> >>>
>> >>> Yes, there are some unrecoverable errors. But not all of them are,
>> such
>> >>> as
>> >>> a subset of the KeeperExceptions around node state, security, and
>> >>> versions.
>> >>> I could be sold on a split, where those type of items are exposed and
>> >>> then
>> >>> the critical ones you keep mentioning are Runtime. But as much as I
>> >>> dislike
>> >>> generic Exceptions for everything, forcing users to catch
>> >>> RuntimeExceptions
>> >>> to do proper exception handling for known and well defined exceptions
>> is
>> >>> an
>> >>> awful practice to put people though.
>> >>>
>> >>>
>> >>> On Fri, Aug 1, 2014 at 3:49 PM, Jordan Zimmerman <
>> >>> jordan@jordanzimmerman.com
>> >>> > wrote:
>> >>>
>> >>> > -1 (binding)
>> >>> >
>> >>> > If I could go back I’d remove all checked exceptions entirely. I
>> don’t
>> >>> > think there’s an objective answer here - it comes down to personal
>> >>> > preference, etc. I don’t see much value in touching nearly every
>> file
>> >>> in
>> >>> > the library in order to do this. We’ve had maybe 2 or 3 requests in
>> the
>> >>> > many years that Curator has exists. This suggests that the
>> overwhelming
>> >>> > majority accept the current exception semantics. If you can point
>> to an
>> >>> > actual bug that this causes that would be helpful.
>> >>> >
>> >>> > -Jordan
>> >>> >
>> >>> > From: Mike Drob <ma...@cloudera.com>
>> >>> > Reply: dev@curator.apache.org <de...@curator.apache.org>>
>> >>> > Date: August 1, 2014 at 2:32:07 PM
>> >>> > To: dev@curator.apache.org <de...@curator.apache.org>>
>> >>> > Subject: Exception throwing
>> >>> >
>> >>> > I'd like to revisit the discussion around always throwing Exception
>> in
>> >>> the
>> >>> > API. There were two JIRA issues - CURATOR-135 and CURATOR-29 - that
>> >>> touch
>> >>> > on this subject, but I think there is a good conversation to be had.
>> >>> >
>> >>> > I understand the suggestions that if an exception is thrown, we are
>> in
>> >>> a
>> >>> > bad state, regardless of the type of exception. However, throwing
>> >>> Exception
>> >>> > comes with some unfortunate Java baggage...
>> >>> >
>> >>> > By declaring thrown Exception, we force consumers to also catch
>> >>> > RuntimeExceptions instead of letting them propagate as they normally
>> >>> would.
>> >>> > In some cases, the calling code may need to attempt roll-back
>> >>> semantics, or
>> >>> > retry outside of what Curator provides, or something else that we
>> >>> haven't
>> >>> > thought of.
>> >>> >
>> >>> > I'd like to propose replacing much of the thrown Exception methods
>> with
>> >>> > CuratorException. This will still carry the connotation that it
>> doesn't
>> >>> > matter what kind of exception we encounter, they're all bad. It will
>> >>> also
>> >>> > be backwards compatible with the previous API, since users will
>> still
>> >>> be
>> >>> > able to catch Exception in their calling code. And it has the
>> >>> advantage of
>> >>> > separating checked exceptions from unchecked ones, so that users
>> don't
>> >>> > unintentionally catch something unrelated.
>> >>> >
>> >>> > Thoughts?
>> >>> >
>> >>> > I tried looking for more details behind the design decision to
>> always
>> >>> throw
>> >>> > Exception, but wasn't able to find them. If they're already
>> >>> documented, I'd
>> >>> > love to be pointed at the wiki or site, or a mailing list thread
>> will
>> >>> do in
>> >>> > a pinch.
>> >>> >
>> >>> > Thanks,
>> >>> > Mike
>> >>> >
>> >>>
>> >>
>> >>
>> >
>>
>
>
Re: Exception throwing
Posted by Mike Drob <ma...@cloudera.com>.
I can see how it may be unappealing modifying everything in the library to
accommodate this. And I think I figured out why this matters (before I was
suggesting it from mostly intuition).
Jordan, you posit that Curator assumes familiarity with ZooKeeper. In some
cases, that's warranted, but in others, I'm not so sure. I understand the
basic ZooKeeper semantics - you can put and get data, it provides quorum
guarantees, data is stored hierarchically, etc... However, I don't know the
ins and outs of the ZooKeeper API. I know that KeeperException is a thing,
but I couldn't tell you which methods throw it, why they throw it, and what
all the (enum? int? string?) codes are. Dealing directly with ZooKeeper is
thorny!
Enter Curator! It gives me so much for free - retry policies, sanity
checking, a testing server for unit tests, it's great. Until I run into a
KeeperException. That's a very jarring experience because I thought that
Curator protected me from such nonsense. And it never advertised that
KeeperException was a possibility! My only indication was the 'throws
Exception' clause on most methods, which I have been taught by the library
to dutifully ignore it and just fail fast. Had I seen a 'throws
KeeperException' declaration, I might have thought about what that means,
but that information was not available to me.
Yes, it is unfortunate that ZooKeeper uses exceptions to return result
codes. That doesn't mean that we have to do the same. I would love to see
CreateBuilder implement BackgroundPathAndBytesable<Boolean> instead of
String and for it to eat the KeeperException. Or maybe return a Stat. Just
protect me from that exception.
Mike
On Sat, Aug 2, 2014 at 5:47 PM, Cameron McKenzie <mc...@gmail.com>
wrote:
> I don't have a super strong view either way. If we were designing from
> scratch I'd go with checked exceptions, just because that's how I prefer to
> code.
>
> Having said that, I'm not convinced that modifying the whole code base at
> this point to add a CuratorException is worth the effort.
>
>
> On Sat, Aug 2, 2014 at 8:09 AM, Jordan Zimmerman <
> jordan@jordanzimmerman.com> wrote:
>
>> Curator assumes familiarity with ZooKeeper. Of course, the docs should be
>> improved where needed. Curator doesn’t hide any KeeperExceptions except
>> where it advertises that it handles them - e.g. creatingParentsIfNeeded().
>> The fact that ZooKeeper uses exceptions to return result codes for correct
>> behavior is the problem here.
>>
>> That said, I wrote nearly all of the Curator recipes and have never had
>> an occasion where Curator’s use of Exception was a problem.
>>
>> -JZ
>>
>> From: Mike Drob <ma...@cloudera.com>
>> Reply: dev@curator.apache.org <de...@curator.apache.org>>
>> Date: August 1, 2014 at 5:05:04 PM
>> To: vines@apache.org <vi...@apache.org>>
>> Cc: dev@curator.apache.org <de...@curator.apache.org>>
>> Subject: Re: Exception throwing
>>
>> The set with version is basically a compareAndSet. java.util chooses to
>> implement this also with a 'return false' value for when some other thread
>> got there first. I'll have to go back and look at the Curator API and see
>> how these failures are currently communicated.
>>
>>
>> On Fri, Aug 1, 2014 at 4:08 PM, John Vines <vi...@apache.org> wrote:
>>
>> > There's KeeperException.BADVERSION, which is when you
>> > setData().withVersion and the version in ZK changed vs. the version seen
>> > prior. That one is pretty critical to support, IMO. The other cases
>> around
>> > node existance can definitely be handled by the user, but given the
>> > possibilities for races in distributed systems you still can't be
>> certain.
>> > But there could be user cases where they want to create a node and not
>> fail
>> > if it exists or have a delete pass if the node was already deleted by
>> > something else (not sure if a flag was added for that case, I vaguely
>> > recall a discussion).
>> >
>> >
>> > On Fri, Aug 1, 2014 at 5:04 PM, Mike Drob <ma...@cloudera.com> wrote:
>> >
>> >> John, thanks for your input. So some of this is improper use of the
>> API,
>> >> right? If you are attempting to create a node and it already exists,
>> then
>> >> that can be an exceptional case. If you just want to make sure that a
>> node
>> >> exists, regardless of who created it, then that's a case for a
>> different
>> >> API. Asserting that you created the node could be important - see the
>> >> distinction in ConcurrentMap between put() and putIfAbsent(). Then
>> again,
>> >> failing to create the node because it already exists might not need to
>> be
>> >> an exceptional case and simply warrants a 'return false' on the
>> method? Do
>> >> the other cases you mentioned have similar analogues? Maybe the end
>> result
>> >> of what comes out of this is better docs.
>> >>
>> >> Mike
>> >>
>> >>
>> >> On Fri, Aug 1, 2014 at 3:39 PM, John Vines <vi...@apache.org> wrote:
>> >>
>> >>> It's not a matter of it being a bug, it's a matter of usability.
>> Because
>> >>> every single method just throws Exception it gives me, as a user,
>> >>> absolutely zero inclination at writing time to figure out what sort of
>> >>> failures can happen. And the complete lack of javadocs compound this
>> >>> issue.
>> >>> This has been my biggest issue with Curator.
>> >>>
>> >>> Yes, there are some unrecoverable errors. But not all of them are,
>> such
>> >>> as
>> >>> a subset of the KeeperExceptions around node state, security, and
>> >>> versions.
>> >>> I could be sold on a split, where those type of items are exposed and
>> >>> then
>> >>> the critical ones you keep mentioning are Runtime. But as much as I
>> >>> dislike
>> >>> generic Exceptions for everything, forcing users to catch
>> >>> RuntimeExceptions
>> >>> to do proper exception handling for known and well defined exceptions
>> is
>> >>> an
>> >>> awful practice to put people though.
>> >>>
>> >>>
>> >>> On Fri, Aug 1, 2014 at 3:49 PM, Jordan Zimmerman <
>> >>> jordan@jordanzimmerman.com
>> >>> > wrote:
>> >>>
>> >>> > -1 (binding)
>> >>> >
>> >>> > If I could go back I’d remove all checked exceptions entirely. I
>> don’t
>> >>> > think there’s an objective answer here - it comes down to personal
>> >>> > preference, etc. I don’t see much value in touching nearly every
>> file
>> >>> in
>> >>> > the library in order to do this. We’ve had maybe 2 or 3 requests in
>> the
>> >>> > many years that Curator has exists. This suggests that the
>> overwhelming
>> >>> > majority accept the current exception semantics. If you can point
>> to an
>> >>> > actual bug that this causes that would be helpful.
>> >>> >
>> >>> > -Jordan
>> >>> >
>> >>> > From: Mike Drob <ma...@cloudera.com>
>> >>> > Reply: dev@curator.apache.org <de...@curator.apache.org>>
>> >>> > Date: August 1, 2014 at 2:32:07 PM
>> >>> > To: dev@curator.apache.org <de...@curator.apache.org>>
>> >>> > Subject: Exception throwing
>> >>> >
>> >>> > I'd like to revisit the discussion around always throwing Exception
>> in
>> >>> the
>> >>> > API. There were two JIRA issues - CURATOR-135 and CURATOR-29 - that
>> >>> touch
>> >>> > on this subject, but I think there is a good conversation to be had.
>> >>> >
>> >>> > I understand the suggestions that if an exception is thrown, we are
>> in
>> >>> a
>> >>> > bad state, regardless of the type of exception. However, throwing
>> >>> Exception
>> >>> > comes with some unfortunate Java baggage...
>> >>> >
>> >>> > By declaring thrown Exception, we force consumers to also catch
>> >>> > RuntimeExceptions instead of letting them propagate as they normally
>> >>> would.
>> >>> > In some cases, the calling code may need to attempt roll-back
>> >>> semantics, or
>> >>> > retry outside of what Curator provides, or something else that we
>> >>> haven't
>> >>> > thought of.
>> >>> >
>> >>> > I'd like to propose replacing much of the thrown Exception methods
>> with
>> >>> > CuratorException. This will still carry the connotation that it
>> doesn't
>> >>> > matter what kind of exception we encounter, they're all bad. It will
>> >>> also
>> >>> > be backwards compatible with the previous API, since users will
>> still
>> >>> be
>> >>> > able to catch Exception in their calling code. And it has the
>> >>> advantage of
>> >>> > separating checked exceptions from unchecked ones, so that users
>> don't
>> >>> > unintentionally catch something unrelated.
>> >>> >
>> >>> > Thoughts?
>> >>> >
>> >>> > I tried looking for more details behind the design decision to
>> always
>> >>> throw
>> >>> > Exception, but wasn't able to find them. If they're already
>> >>> documented, I'd
>> >>> > love to be pointed at the wiki or site, or a mailing list thread
>> will
>> >>> do in
>> >>> > a pinch.
>> >>> >
>> >>> > Thanks,
>> >>> > Mike
>> >>> >
>> >>>
>> >>
>> >>
>> >
>>
>
>
Re: Exception throwing
Posted by Cameron McKenzie <mc...@gmail.com>.
I don't have a super strong view either way. If we were designing from
scratch I'd go with checked exceptions, just because that's how I prefer to
code.
Having said that, I'm not convinced that modifying the whole code base at
this point to add a CuratorException is worth the effort.
On Sat, Aug 2, 2014 at 8:09 AM, Jordan Zimmerman <jordan@jordanzimmerman.com
> wrote:
> Curator assumes familiarity with ZooKeeper. Of course, the docs should be
> improved where needed. Curator doesn’t hide any KeeperExceptions except
> where it advertises that it handles them - e.g. creatingParentsIfNeeded().
> The fact that ZooKeeper uses exceptions to return result codes for correct
> behavior is the problem here.
>
> That said, I wrote nearly all of the Curator recipes and have never had an
> occasion where Curator’s use of Exception was a problem.
>
> -JZ
>
> From: Mike Drob <ma...@cloudera.com>
> Reply: dev@curator.apache.org <de...@curator.apache.org>>
> Date: August 1, 2014 at 5:05:04 PM
> To: vines@apache.org <vi...@apache.org>>
> Cc: dev@curator.apache.org <de...@curator.apache.org>>
> Subject: Re: Exception throwing
>
> The set with version is basically a compareAndSet. java.util chooses to
> implement this also with a 'return false' value for when some other thread
> got there first. I'll have to go back and look at the Curator API and see
> how these failures are currently communicated.
>
>
> On Fri, Aug 1, 2014 at 4:08 PM, John Vines <vi...@apache.org> wrote:
>
> > There's KeeperException.BADVERSION, which is when you
> > setData().withVersion and the version in ZK changed vs. the version seen
> > prior. That one is pretty critical to support, IMO. The other cases
> around
> > node existance can definitely be handled by the user, but given the
> > possibilities for races in distributed systems you still can't be
> certain.
> > But there could be user cases where they want to create a node and not
> fail
> > if it exists or have a delete pass if the node was already deleted by
> > something else (not sure if a flag was added for that case, I vaguely
> > recall a discussion).
> >
> >
> > On Fri, Aug 1, 2014 at 5:04 PM, Mike Drob <ma...@cloudera.com> wrote:
> >
> >> John, thanks for your input. So some of this is improper use of the API,
> >> right? If you are attempting to create a node and it already exists,
> then
> >> that can be an exceptional case. If you just want to make sure that a
> node
> >> exists, regardless of who created it, then that's a case for a different
> >> API. Asserting that you created the node could be important - see the
> >> distinction in ConcurrentMap between put() and putIfAbsent(). Then
> again,
> >> failing to create the node because it already exists might not need to
> be
> >> an exceptional case and simply warrants a 'return false' on the method?
> Do
> >> the other cases you mentioned have similar analogues? Maybe the end
> result
> >> of what comes out of this is better docs.
> >>
> >> Mike
> >>
> >>
> >> On Fri, Aug 1, 2014 at 3:39 PM, John Vines <vi...@apache.org> wrote:
> >>
> >>> It's not a matter of it being a bug, it's a matter of usability.
> Because
> >>> every single method just throws Exception it gives me, as a user,
> >>> absolutely zero inclination at writing time to figure out what sort of
> >>> failures can happen. And the complete lack of javadocs compound this
> >>> issue.
> >>> This has been my biggest issue with Curator.
> >>>
> >>> Yes, there are some unrecoverable errors. But not all of them are, such
> >>> as
> >>> a subset of the KeeperExceptions around node state, security, and
> >>> versions.
> >>> I could be sold on a split, where those type of items are exposed and
> >>> then
> >>> the critical ones you keep mentioning are Runtime. But as much as I
> >>> dislike
> >>> generic Exceptions for everything, forcing users to catch
> >>> RuntimeExceptions
> >>> to do proper exception handling for known and well defined exceptions
> is
> >>> an
> >>> awful practice to put people though.
> >>>
> >>>
> >>> On Fri, Aug 1, 2014 at 3:49 PM, Jordan Zimmerman <
> >>> jordan@jordanzimmerman.com
> >>> > wrote:
> >>>
> >>> > -1 (binding)
> >>> >
> >>> > If I could go back I’d remove all checked exceptions entirely. I
> don’t
> >>> > think there’s an objective answer here - it comes down to personal
> >>> > preference, etc. I don’t see much value in touching nearly every file
> >>> in
> >>> > the library in order to do this. We’ve had maybe 2 or 3 requests in
> the
> >>> > many years that Curator has exists. This suggests that the
> overwhelming
> >>> > majority accept the current exception semantics. If you can point to
> an
> >>> > actual bug that this causes that would be helpful.
> >>> >
> >>> > -Jordan
> >>> >
> >>> > From: Mike Drob <ma...@cloudera.com>
> >>> > Reply: dev@curator.apache.org <de...@curator.apache.org>>
> >>> > Date: August 1, 2014 at 2:32:07 PM
> >>> > To: dev@curator.apache.org <de...@curator.apache.org>>
> >>> > Subject: Exception throwing
> >>> >
> >>> > I'd like to revisit the discussion around always throwing Exception
> in
> >>> the
> >>> > API. There were two JIRA issues - CURATOR-135 and CURATOR-29 - that
> >>> touch
> >>> > on this subject, but I think there is a good conversation to be had.
> >>> >
> >>> > I understand the suggestions that if an exception is thrown, we are
> in
> >>> a
> >>> > bad state, regardless of the type of exception. However, throwing
> >>> Exception
> >>> > comes with some unfortunate Java baggage...
> >>> >
> >>> > By declaring thrown Exception, we force consumers to also catch
> >>> > RuntimeExceptions instead of letting them propagate as they normally
> >>> would.
> >>> > In some cases, the calling code may need to attempt roll-back
> >>> semantics, or
> >>> > retry outside of what Curator provides, or something else that we
> >>> haven't
> >>> > thought of.
> >>> >
> >>> > I'd like to propose replacing much of the thrown Exception methods
> with
> >>> > CuratorException. This will still carry the connotation that it
> doesn't
> >>> > matter what kind of exception we encounter, they're all bad. It will
> >>> also
> >>> > be backwards compatible with the previous API, since users will still
> >>> be
> >>> > able to catch Exception in their calling code. And it has the
> >>> advantage of
> >>> > separating checked exceptions from unchecked ones, so that users
> don't
> >>> > unintentionally catch something unrelated.
> >>> >
> >>> > Thoughts?
> >>> >
> >>> > I tried looking for more details behind the design decision to always
> >>> throw
> >>> > Exception, but wasn't able to find them. If they're already
> >>> documented, I'd
> >>> > love to be pointed at the wiki or site, or a mailing list thread will
> >>> do in
> >>> > a pinch.
> >>> >
> >>> > Thanks,
> >>> > Mike
> >>> >
> >>>
> >>
> >>
> >
>
Re: Exception throwing
Posted by Jordan Zimmerman <jo...@jordanzimmerman.com>.
Curator assumes familiarity with ZooKeeper. Of course, the docs should be improved where needed. Curator doesn’t hide any KeeperExceptions except where it advertises that it handles them - e.g. creatingParentsIfNeeded(). The fact that ZooKeeper uses exceptions to return result codes for correct behavior is the problem here.
That said, I wrote nearly all of the Curator recipes and have never had an occasion where Curator’s use of Exception was a problem.
-JZ
From: Mike Drob <ma...@cloudera.com>
Reply: dev@curator.apache.org <de...@curator.apache.org>>
Date: August 1, 2014 at 5:05:04 PM
To: vines@apache.org <vi...@apache.org>>
Cc: dev@curator.apache.org <de...@curator.apache.org>>
Subject: Re: Exception throwing
The set with version is basically a compareAndSet. java.util chooses to
implement this also with a 'return false' value for when some other thread
got there first. I'll have to go back and look at the Curator API and see
how these failures are currently communicated.
On Fri, Aug 1, 2014 at 4:08 PM, John Vines <vi...@apache.org> wrote:
> There's KeeperException.BADVERSION, which is when you
> setData().withVersion and the version in ZK changed vs. the version seen
> prior. That one is pretty critical to support, IMO. The other cases around
> node existance can definitely be handled by the user, but given the
> possibilities for races in distributed systems you still can't be certain.
> But there could be user cases where they want to create a node and not fail
> if it exists or have a delete pass if the node was already deleted by
> something else (not sure if a flag was added for that case, I vaguely
> recall a discussion).
>
>
> On Fri, Aug 1, 2014 at 5:04 PM, Mike Drob <ma...@cloudera.com> wrote:
>
>> John, thanks for your input. So some of this is improper use of the API,
>> right? If you are attempting to create a node and it already exists, then
>> that can be an exceptional case. If you just want to make sure that a node
>> exists, regardless of who created it, then that's a case for a different
>> API. Asserting that you created the node could be important - see the
>> distinction in ConcurrentMap between put() and putIfAbsent(). Then again,
>> failing to create the node because it already exists might not need to be
>> an exceptional case and simply warrants a 'return false' on the method? Do
>> the other cases you mentioned have similar analogues? Maybe the end result
>> of what comes out of this is better docs.
>>
>> Mike
>>
>>
>> On Fri, Aug 1, 2014 at 3:39 PM, John Vines <vi...@apache.org> wrote:
>>
>>> It's not a matter of it being a bug, it's a matter of usability. Because
>>> every single method just throws Exception it gives me, as a user,
>>> absolutely zero inclination at writing time to figure out what sort of
>>> failures can happen. And the complete lack of javadocs compound this
>>> issue.
>>> This has been my biggest issue with Curator.
>>>
>>> Yes, there are some unrecoverable errors. But not all of them are, such
>>> as
>>> a subset of the KeeperExceptions around node state, security, and
>>> versions.
>>> I could be sold on a split, where those type of items are exposed and
>>> then
>>> the critical ones you keep mentioning are Runtime. But as much as I
>>> dislike
>>> generic Exceptions for everything, forcing users to catch
>>> RuntimeExceptions
>>> to do proper exception handling for known and well defined exceptions is
>>> an
>>> awful practice to put people though.
>>>
>>>
>>> On Fri, Aug 1, 2014 at 3:49 PM, Jordan Zimmerman <
>>> jordan@jordanzimmerman.com
>>> > wrote:
>>>
>>> > -1 (binding)
>>> >
>>> > If I could go back I’d remove all checked exceptions entirely. I don’t
>>> > think there’s an objective answer here - it comes down to personal
>>> > preference, etc. I don’t see much value in touching nearly every file
>>> in
>>> > the library in order to do this. We’ve had maybe 2 or 3 requests in the
>>> > many years that Curator has exists. This suggests that the overwhelming
>>> > majority accept the current exception semantics. If you can point to an
>>> > actual bug that this causes that would be helpful.
>>> >
>>> > -Jordan
>>> >
>>> > From: Mike Drob <ma...@cloudera.com>
>>> > Reply: dev@curator.apache.org <de...@curator.apache.org>>
>>> > Date: August 1, 2014 at 2:32:07 PM
>>> > To: dev@curator.apache.org <de...@curator.apache.org>>
>>> > Subject: Exception throwing
>>> >
>>> > I'd like to revisit the discussion around always throwing Exception in
>>> the
>>> > API. There were two JIRA issues - CURATOR-135 and CURATOR-29 - that
>>> touch
>>> > on this subject, but I think there is a good conversation to be had.
>>> >
>>> > I understand the suggestions that if an exception is thrown, we are in
>>> a
>>> > bad state, regardless of the type of exception. However, throwing
>>> Exception
>>> > comes with some unfortunate Java baggage...
>>> >
>>> > By declaring thrown Exception, we force consumers to also catch
>>> > RuntimeExceptions instead of letting them propagate as they normally
>>> would.
>>> > In some cases, the calling code may need to attempt roll-back
>>> semantics, or
>>> > retry outside of what Curator provides, or something else that we
>>> haven't
>>> > thought of.
>>> >
>>> > I'd like to propose replacing much of the thrown Exception methods with
>>> > CuratorException. This will still carry the connotation that it doesn't
>>> > matter what kind of exception we encounter, they're all bad. It will
>>> also
>>> > be backwards compatible with the previous API, since users will still
>>> be
>>> > able to catch Exception in their calling code. And it has the
>>> advantage of
>>> > separating checked exceptions from unchecked ones, so that users don't
>>> > unintentionally catch something unrelated.
>>> >
>>> > Thoughts?
>>> >
>>> > I tried looking for more details behind the design decision to always
>>> throw
>>> > Exception, but wasn't able to find them. If they're already
>>> documented, I'd
>>> > love to be pointed at the wiki or site, or a mailing list thread will
>>> do in
>>> > a pinch.
>>> >
>>> > Thanks,
>>> > Mike
>>> >
>>>
>>
>>
>
Re: Exception throwing
Posted by Mike Drob <ma...@cloudera.com>.
The set with version is basically a compareAndSet. java.util chooses to
implement this also with a 'return false' value for when some other thread
got there first. I'll have to go back and look at the Curator API and see
how these failures are currently communicated.
On Fri, Aug 1, 2014 at 4:08 PM, John Vines <vi...@apache.org> wrote:
> There's KeeperException.BADVERSION, which is when you
> setData().withVersion and the version in ZK changed vs. the version seen
> prior. That one is pretty critical to support, IMO. The other cases around
> node existance can definitely be handled by the user, but given the
> possibilities for races in distributed systems you still can't be certain.
> But there could be user cases where they want to create a node and not fail
> if it exists or have a delete pass if the node was already deleted by
> something else (not sure if a flag was added for that case, I vaguely
> recall a discussion).
>
>
> On Fri, Aug 1, 2014 at 5:04 PM, Mike Drob <ma...@cloudera.com> wrote:
>
>> John, thanks for your input. So some of this is improper use of the API,
>> right? If you are attempting to create a node and it already exists, then
>> that can be an exceptional case. If you just want to make sure that a node
>> exists, regardless of who created it, then that's a case for a different
>> API. Asserting that you created the node could be important - see the
>> distinction in ConcurrentMap between put() and putIfAbsent(). Then again,
>> failing to create the node because it already exists might not need to be
>> an exceptional case and simply warrants a 'return false' on the method? Do
>> the other cases you mentioned have similar analogues? Maybe the end result
>> of what comes out of this is better docs.
>>
>> Mike
>>
>>
>> On Fri, Aug 1, 2014 at 3:39 PM, John Vines <vi...@apache.org> wrote:
>>
>>> It's not a matter of it being a bug, it's a matter of usability. Because
>>> every single method just throws Exception it gives me, as a user,
>>> absolutely zero inclination at writing time to figure out what sort of
>>> failures can happen. And the complete lack of javadocs compound this
>>> issue.
>>> This has been my biggest issue with Curator.
>>>
>>> Yes, there are some unrecoverable errors. But not all of them are, such
>>> as
>>> a subset of the KeeperExceptions around node state, security, and
>>> versions.
>>> I could be sold on a split, where those type of items are exposed and
>>> then
>>> the critical ones you keep mentioning are Runtime. But as much as I
>>> dislike
>>> generic Exceptions for everything, forcing users to catch
>>> RuntimeExceptions
>>> to do proper exception handling for known and well defined exceptions is
>>> an
>>> awful practice to put people though.
>>>
>>>
>>> On Fri, Aug 1, 2014 at 3:49 PM, Jordan Zimmerman <
>>> jordan@jordanzimmerman.com
>>> > wrote:
>>>
>>> > -1 (binding)
>>> >
>>> > If I could go back I’d remove all checked exceptions entirely. I don’t
>>> > think there’s an objective answer here - it comes down to personal
>>> > preference, etc. I don’t see much value in touching nearly every file
>>> in
>>> > the library in order to do this. We’ve had maybe 2 or 3 requests in the
>>> > many years that Curator has exists. This suggests that the overwhelming
>>> > majority accept the current exception semantics. If you can point to an
>>> > actual bug that this causes that would be helpful.
>>> >
>>> > -Jordan
>>> >
>>> > From: Mike Drob <ma...@cloudera.com>
>>> > Reply: dev@curator.apache.org <de...@curator.apache.org>>
>>> > Date: August 1, 2014 at 2:32:07 PM
>>> > To: dev@curator.apache.org <de...@curator.apache.org>>
>>> > Subject: Exception throwing
>>> >
>>> > I'd like to revisit the discussion around always throwing Exception in
>>> the
>>> > API. There were two JIRA issues - CURATOR-135 and CURATOR-29 - that
>>> touch
>>> > on this subject, but I think there is a good conversation to be had.
>>> >
>>> > I understand the suggestions that if an exception is thrown, we are in
>>> a
>>> > bad state, regardless of the type of exception. However, throwing
>>> Exception
>>> > comes with some unfortunate Java baggage...
>>> >
>>> > By declaring thrown Exception, we force consumers to also catch
>>> > RuntimeExceptions instead of letting them propagate as they normally
>>> would.
>>> > In some cases, the calling code may need to attempt roll-back
>>> semantics, or
>>> > retry outside of what Curator provides, or something else that we
>>> haven't
>>> > thought of.
>>> >
>>> > I'd like to propose replacing much of the thrown Exception methods with
>>> > CuratorException. This will still carry the connotation that it doesn't
>>> > matter what kind of exception we encounter, they're all bad. It will
>>> also
>>> > be backwards compatible with the previous API, since users will still
>>> be
>>> > able to catch Exception in their calling code. And it has the
>>> advantage of
>>> > separating checked exceptions from unchecked ones, so that users don't
>>> > unintentionally catch something unrelated.
>>> >
>>> > Thoughts?
>>> >
>>> > I tried looking for more details behind the design decision to always
>>> throw
>>> > Exception, but wasn't able to find them. If they're already
>>> documented, I'd
>>> > love to be pointed at the wiki or site, or a mailing list thread will
>>> do in
>>> > a pinch.
>>> >
>>> > Thanks,
>>> > Mike
>>> >
>>>
>>
>>
>
Re: Exception throwing
Posted by John Vines <vi...@apache.org>.
There's KeeperException.BADVERSION, which is when you setData().withVersion
and the version in ZK changed vs. the version seen prior. That one is
pretty critical to support, IMO. The other cases around node existance can
definitely be handled by the user, but given the possibilities for races in
distributed systems you still can't be certain. But there could be user
cases where they want to create a node and not fail if it exists or have a
delete pass if the node was already deleted by something else (not sure if
a flag was added for that case, I vaguely recall a discussion).
On Fri, Aug 1, 2014 at 5:04 PM, Mike Drob <ma...@cloudera.com> wrote:
> John, thanks for your input. So some of this is improper use of the API,
> right? If you are attempting to create a node and it already exists, then
> that can be an exceptional case. If you just want to make sure that a node
> exists, regardless of who created it, then that's a case for a different
> API. Asserting that you created the node could be important - see the
> distinction in ConcurrentMap between put() and putIfAbsent(). Then again,
> failing to create the node because it already exists might not need to be
> an exceptional case and simply warrants a 'return false' on the method? Do
> the other cases you mentioned have similar analogues? Maybe the end result
> of what comes out of this is better docs.
>
> Mike
>
>
> On Fri, Aug 1, 2014 at 3:39 PM, John Vines <vi...@apache.org> wrote:
>
>> It's not a matter of it being a bug, it's a matter of usability. Because
>> every single method just throws Exception it gives me, as a user,
>> absolutely zero inclination at writing time to figure out what sort of
>> failures can happen. And the complete lack of javadocs compound this
>> issue.
>> This has been my biggest issue with Curator.
>>
>> Yes, there are some unrecoverable errors. But not all of them are, such as
>> a subset of the KeeperExceptions around node state, security, and
>> versions.
>> I could be sold on a split, where those type of items are exposed and then
>> the critical ones you keep mentioning are Runtime. But as much as I
>> dislike
>> generic Exceptions for everything, forcing users to catch
>> RuntimeExceptions
>> to do proper exception handling for known and well defined exceptions is
>> an
>> awful practice to put people though.
>>
>>
>> On Fri, Aug 1, 2014 at 3:49 PM, Jordan Zimmerman <
>> jordan@jordanzimmerman.com
>> > wrote:
>>
>> > -1 (binding)
>> >
>> > If I could go back I’d remove all checked exceptions entirely. I don’t
>> > think there’s an objective answer here - it comes down to personal
>> > preference, etc. I don’t see much value in touching nearly every file in
>> > the library in order to do this. We’ve had maybe 2 or 3 requests in the
>> > many years that Curator has exists. This suggests that the overwhelming
>> > majority accept the current exception semantics. If you can point to an
>> > actual bug that this causes that would be helpful.
>> >
>> > -Jordan
>> >
>> > From: Mike Drob <ma...@cloudera.com>
>> > Reply: dev@curator.apache.org <de...@curator.apache.org>>
>> > Date: August 1, 2014 at 2:32:07 PM
>> > To: dev@curator.apache.org <de...@curator.apache.org>>
>> > Subject: Exception throwing
>> >
>> > I'd like to revisit the discussion around always throwing Exception in
>> the
>> > API. There were two JIRA issues - CURATOR-135 and CURATOR-29 - that
>> touch
>> > on this subject, but I think there is a good conversation to be had.
>> >
>> > I understand the suggestions that if an exception is thrown, we are in a
>> > bad state, regardless of the type of exception. However, throwing
>> Exception
>> > comes with some unfortunate Java baggage...
>> >
>> > By declaring thrown Exception, we force consumers to also catch
>> > RuntimeExceptions instead of letting them propagate as they normally
>> would.
>> > In some cases, the calling code may need to attempt roll-back
>> semantics, or
>> > retry outside of what Curator provides, or something else that we
>> haven't
>> > thought of.
>> >
>> > I'd like to propose replacing much of the thrown Exception methods with
>> > CuratorException. This will still carry the connotation that it doesn't
>> > matter what kind of exception we encounter, they're all bad. It will
>> also
>> > be backwards compatible with the previous API, since users will still be
>> > able to catch Exception in their calling code. And it has the advantage
>> of
>> > separating checked exceptions from unchecked ones, so that users don't
>> > unintentionally catch something unrelated.
>> >
>> > Thoughts?
>> >
>> > I tried looking for more details behind the design decision to always
>> throw
>> > Exception, but wasn't able to find them. If they're already documented,
>> I'd
>> > love to be pointed at the wiki or site, or a mailing list thread will
>> do in
>> > a pinch.
>> >
>> > Thanks,
>> > Mike
>> >
>>
>
>
Re: Exception throwing
Posted by Mike Drob <ma...@cloudera.com>.
John, thanks for your input. So some of this is improper use of the API,
right? If you are attempting to create a node and it already exists, then
that can be an exceptional case. If you just want to make sure that a node
exists, regardless of who created it, then that's a case for a different
API. Asserting that you created the node could be important - see the
distinction in ConcurrentMap between put() and putIfAbsent(). Then again,
failing to create the node because it already exists might not need to be
an exceptional case and simply warrants a 'return false' on the method? Do
the other cases you mentioned have similar analogues? Maybe the end result
of what comes out of this is better docs.
Mike
On Fri, Aug 1, 2014 at 3:39 PM, John Vines <vi...@apache.org> wrote:
> It's not a matter of it being a bug, it's a matter of usability. Because
> every single method just throws Exception it gives me, as a user,
> absolutely zero inclination at writing time to figure out what sort of
> failures can happen. And the complete lack of javadocs compound this issue.
> This has been my biggest issue with Curator.
>
> Yes, there are some unrecoverable errors. But not all of them are, such as
> a subset of the KeeperExceptions around node state, security, and versions.
> I could be sold on a split, where those type of items are exposed and then
> the critical ones you keep mentioning are Runtime. But as much as I dislike
> generic Exceptions for everything, forcing users to catch RuntimeExceptions
> to do proper exception handling for known and well defined exceptions is an
> awful practice to put people though.
>
>
> On Fri, Aug 1, 2014 at 3:49 PM, Jordan Zimmerman <
> jordan@jordanzimmerman.com
> > wrote:
>
> > -1 (binding)
> >
> > If I could go back I’d remove all checked exceptions entirely. I don’t
> > think there’s an objective answer here - it comes down to personal
> > preference, etc. I don’t see much value in touching nearly every file in
> > the library in order to do this. We’ve had maybe 2 or 3 requests in the
> > many years that Curator has exists. This suggests that the overwhelming
> > majority accept the current exception semantics. If you can point to an
> > actual bug that this causes that would be helpful.
> >
> > -Jordan
> >
> > From: Mike Drob <ma...@cloudera.com>
> > Reply: dev@curator.apache.org <de...@curator.apache.org>>
> > Date: August 1, 2014 at 2:32:07 PM
> > To: dev@curator.apache.org <de...@curator.apache.org>>
> > Subject: Exception throwing
> >
> > I'd like to revisit the discussion around always throwing Exception in
> the
> > API. There were two JIRA issues - CURATOR-135 and CURATOR-29 - that touch
> > on this subject, but I think there is a good conversation to be had.
> >
> > I understand the suggestions that if an exception is thrown, we are in a
> > bad state, regardless of the type of exception. However, throwing
> Exception
> > comes with some unfortunate Java baggage...
> >
> > By declaring thrown Exception, we force consumers to also catch
> > RuntimeExceptions instead of letting them propagate as they normally
> would.
> > In some cases, the calling code may need to attempt roll-back semantics,
> or
> > retry outside of what Curator provides, or something else that we haven't
> > thought of.
> >
> > I'd like to propose replacing much of the thrown Exception methods with
> > CuratorException. This will still carry the connotation that it doesn't
> > matter what kind of exception we encounter, they're all bad. It will also
> > be backwards compatible with the previous API, since users will still be
> > able to catch Exception in their calling code. And it has the advantage
> of
> > separating checked exceptions from unchecked ones, so that users don't
> > unintentionally catch something unrelated.
> >
> > Thoughts?
> >
> > I tried looking for more details behind the design decision to always
> throw
> > Exception, but wasn't able to find them. If they're already documented,
> I'd
> > love to be pointed at the wiki or site, or a mailing list thread will do
> in
> > a pinch.
> >
> > Thanks,
> > Mike
> >
>
Re: Exception throwing
Posted by Jordan Zimmerman <jo...@jordanzimmerman.com>.
This is a philosophical issue. If Curator had no throws clauses at all it would be the same. So, getting rid of all the “throws Exception” statements wouldn’t help this. In my opinion, this is all baked into Curator and not worth changing. But, of course, if you can convince the rest of the community maybe we can change it. I’ve reopened CURATOR-135. If you folks can convince the community and one of the other committers than it can get done.
-Jordan
From: John Vines <vi...@apache.org>
Reply: dev@curator.apache.org <de...@curator.apache.org>>, vines@apache.org <vi...@apache.org>>
Date: August 1, 2014 at 3:40:25 PM
To: dev@curator.apache.org <de...@curator.apache.org>>
Cc: Mike Drob <ma...@cloudera.com>>
Subject: Re: Exception throwing
It's not a matter of it being a bug, it's a matter of usability. Because
every single method just throws Exception it gives me, as a user,
absolutely zero inclination at writing time to figure out what sort of
failures can happen. And the complete lack of javadocs compound this issue.
This has been my biggest issue with Curator.
Yes, there are some unrecoverable errors. But not all of them are, such as
a subset of the KeeperExceptions around node state, security, and versions.
I could be sold on a split, where those type of items are exposed and then
the critical ones you keep mentioning are Runtime. But as much as I dislike
generic Exceptions for everything, forcing users to catch RuntimeExceptions
to do proper exception handling for known and well defined exceptions is an
awful practice to put people though.
On Fri, Aug 1, 2014 at 3:49 PM, Jordan Zimmerman <jordan@jordanzimmerman.com
> wrote:
> -1 (binding)
>
> If I could go back I’d remove all checked exceptions entirely. I don’t
> think there’s an objective answer here - it comes down to personal
> preference, etc. I don’t see much value in touching nearly every file in
> the library in order to do this. We’ve had maybe 2 or 3 requests in the
> many years that Curator has exists. This suggests that the overwhelming
> majority accept the current exception semantics. If you can point to an
> actual bug that this causes that would be helpful.
>
> -Jordan
>
> From: Mike Drob <ma...@cloudera.com>
> Reply: dev@curator.apache.org <de...@curator.apache.org>>
> Date: August 1, 2014 at 2:32:07 PM
> To: dev@curator.apache.org <de...@curator.apache.org>>
> Subject: Exception throwing
>
> I'd like to revisit the discussion around always throwing Exception in the
> API. There were two JIRA issues - CURATOR-135 and CURATOR-29 - that touch
> on this subject, but I think there is a good conversation to be had.
>
> I understand the suggestions that if an exception is thrown, we are in a
> bad state, regardless of the type of exception. However, throwing Exception
> comes with some unfortunate Java baggage...
>
> By declaring thrown Exception, we force consumers to also catch
> RuntimeExceptions instead of letting them propagate as they normally would.
> In some cases, the calling code may need to attempt roll-back semantics, or
> retry outside of what Curator provides, or something else that we haven't
> thought of.
>
> I'd like to propose replacing much of the thrown Exception methods with
> CuratorException. This will still carry the connotation that it doesn't
> matter what kind of exception we encounter, they're all bad. It will also
> be backwards compatible with the previous API, since users will still be
> able to catch Exception in their calling code. And it has the advantage of
> separating checked exceptions from unchecked ones, so that users don't
> unintentionally catch something unrelated.
>
> Thoughts?
>
> I tried looking for more details behind the design decision to always throw
> Exception, but wasn't able to find them. If they're already documented, I'd
> love to be pointed at the wiki or site, or a mailing list thread will do in
> a pinch.
>
> Thanks,
> Mike
>
Re: Exception throwing
Posted by John Vines <vi...@apache.org>.
It's not a matter of it being a bug, it's a matter of usability. Because
every single method just throws Exception it gives me, as a user,
absolutely zero inclination at writing time to figure out what sort of
failures can happen. And the complete lack of javadocs compound this issue.
This has been my biggest issue with Curator.
Yes, there are some unrecoverable errors. But not all of them are, such as
a subset of the KeeperExceptions around node state, security, and versions.
I could be sold on a split, where those type of items are exposed and then
the critical ones you keep mentioning are Runtime. But as much as I dislike
generic Exceptions for everything, forcing users to catch RuntimeExceptions
to do proper exception handling for known and well defined exceptions is an
awful practice to put people though.
On Fri, Aug 1, 2014 at 3:49 PM, Jordan Zimmerman <jordan@jordanzimmerman.com
> wrote:
> -1 (binding)
>
> If I could go back I’d remove all checked exceptions entirely. I don’t
> think there’s an objective answer here - it comes down to personal
> preference, etc. I don’t see much value in touching nearly every file in
> the library in order to do this. We’ve had maybe 2 or 3 requests in the
> many years that Curator has exists. This suggests that the overwhelming
> majority accept the current exception semantics. If you can point to an
> actual bug that this causes that would be helpful.
>
> -Jordan
>
> From: Mike Drob <ma...@cloudera.com>
> Reply: dev@curator.apache.org <de...@curator.apache.org>>
> Date: August 1, 2014 at 2:32:07 PM
> To: dev@curator.apache.org <de...@curator.apache.org>>
> Subject: Exception throwing
>
> I'd like to revisit the discussion around always throwing Exception in the
> API. There were two JIRA issues - CURATOR-135 and CURATOR-29 - that touch
> on this subject, but I think there is a good conversation to be had.
>
> I understand the suggestions that if an exception is thrown, we are in a
> bad state, regardless of the type of exception. However, throwing Exception
> comes with some unfortunate Java baggage...
>
> By declaring thrown Exception, we force consumers to also catch
> RuntimeExceptions instead of letting them propagate as they normally would.
> In some cases, the calling code may need to attempt roll-back semantics, or
> retry outside of what Curator provides, or something else that we haven't
> thought of.
>
> I'd like to propose replacing much of the thrown Exception methods with
> CuratorException. This will still carry the connotation that it doesn't
> matter what kind of exception we encounter, they're all bad. It will also
> be backwards compatible with the previous API, since users will still be
> able to catch Exception in their calling code. And it has the advantage of
> separating checked exceptions from unchecked ones, so that users don't
> unintentionally catch something unrelated.
>
> Thoughts?
>
> I tried looking for more details behind the design decision to always throw
> Exception, but wasn't able to find them. If they're already documented, I'd
> love to be pointed at the wiki or site, or a mailing list thread will do in
> a pinch.
>
> Thanks,
> Mike
>
Re: Exception throwing
Posted by Jordan Zimmerman <jo...@jordanzimmerman.com>.
I can’t think of a good reason to do it. It’s fine the way it is now and isn’t causing any problems. It would be very disruptive to change this for little benefit.
-JZ
From: Mike Drob <ma...@cloudera.com>
Reply: Mike Drob <ma...@cloudera.com>>
Date: August 1, 2014 at 3:06:35 PM
To: Jordan Zimmerman <jo...@jordanzimmerman.com>>
Cc: dev@curator.apache.org <de...@curator.apache.org>>
Subject: Re: Exception throwing
So what's preventing us from changing this going forward? It's software, we have the ability to fix it; if something can be improved and we are able to improve it, then we can go ahead and do that.
On Fri, Aug 1, 2014 at 2:55 PM, Jordan Zimmerman <jo...@jordanzimmerman.com> wrote:
Like I said, if I could go back, I’d get rid of checked exceptions entirely.
-JZ
From: Mike Drob <ma...@cloudera.com>
Reply: Mike Drob <ma...@cloudera.com>>
Date: August 1, 2014 at 2:54:47 PM
To: Jordan Zimmerman <jo...@jordanzimmerman.com>>
Cc: dev@curator.apache.org <de...@curator.apache.org>>
Subject: Re: Exception throwing
So why declare that we throw exceptions instead of just throwing everything as a RuntimeException (or subclass thereof)?
On Fri, Aug 1, 2014 at 2:49 PM, Jordan Zimmerman <jo...@jordanzimmerman.com> wrote:
-1 (binding)
If I could go back I’d remove all checked exceptions entirely. I don’t think there’s an objective answer here - it comes down to personal preference, etc. I don’t see much value in touching nearly every file in the library in order to do this. We’ve had maybe 2 or 3 requests in the many years that Curator has exists. This suggests that the overwhelming majority accept the current exception semantics. If you can point to an actual bug that this causes that would be helpful.
-Jordan
From: Mike Drob <ma...@cloudera.com>
Reply: dev@curator.apache.org <de...@curator.apache.org>>
Date: August 1, 2014 at 2:32:07 PM
To: dev@curator.apache.org <de...@curator.apache.org>>
Subject: Exception throwing
I'd like to revisit the discussion around always throwing Exception in the
API. There were two JIRA issues - CURATOR-135 and CURATOR-29 - that touch
on this subject, but I think there is a good conversation to be had.
I understand the suggestions that if an exception is thrown, we are in a
bad state, regardless of the type of exception. However, throwing Exception
comes with some unfortunate Java baggage...
By declaring thrown Exception, we force consumers to also catch
RuntimeExceptions instead of letting them propagate as they normally would.
In some cases, the calling code may need to attempt roll-back semantics, or
retry outside of what Curator provides, or something else that we haven't
thought of.
I'd like to propose replacing much of the thrown Exception methods with
CuratorException. This will still carry the connotation that it doesn't
matter what kind of exception we encounter, they're all bad. It will also
be backwards compatible with the previous API, since users will still be
able to catch Exception in their calling code. And it has the advantage of
separating checked exceptions from unchecked ones, so that users don't
unintentionally catch something unrelated.
Thoughts?
I tried looking for more details behind the design decision to always throw
Exception, but wasn't able to find them. If they're already documented, I'd
love to be pointed at the wiki or site, or a mailing list thread will do in
a pinch.
Thanks,
Mike
Re: Exception throwing
Posted by Mike Drob <ma...@cloudera.com>.
So what's preventing us from changing this going forward? It's software, we
have the ability to fix it; if something can be improved and we are able to
improve it, then we can go ahead and do that.
On Fri, Aug 1, 2014 at 2:55 PM, Jordan Zimmerman <jordan@jordanzimmerman.com
> wrote:
> Like I said, if I could go back, I’d get rid of checked exceptions
> entirely.
>
> -JZ
>
> From: Mike Drob <ma...@cloudera.com> <ma...@cloudera.com>
> Reply: Mike Drob <ma...@cloudera.com>> <ma...@cloudera.com>
> Date: August 1, 2014 at 2:54:47 PM
> To: Jordan Zimmerman <jo...@jordanzimmerman.com>>
> <jo...@jordanzimmerman.com>
> Cc: dev@curator.apache.org <de...@curator.apache.org>>
> <de...@curator.apache.org>
> Subject: Re: Exception throwing
>
> So why declare that we throw exceptions instead of just throwing
> everything as a RuntimeException (or subclass thereof)?
>
>
> On Fri, Aug 1, 2014 at 2:49 PM, Jordan Zimmerman <
> jordan@jordanzimmerman.com> wrote:
>
>> -1 (binding)
>>
>> If I could go back I’d remove all checked exceptions entirely. I don’t
>> think there’s an objective answer here - it comes down to personal
>> preference, etc. I don’t see much value in touching nearly every file in
>> the library in order to do this. We’ve had maybe 2 or 3 requests in the
>> many years that Curator has exists. This suggests that the overwhelming
>> majority accept the current exception semantics. If you can point to an
>> actual bug that this causes that would be helpful.
>>
>> -Jordan
>>
>> From: Mike Drob <ma...@cloudera.com> <ma...@cloudera.com>
>> Reply: dev@curator.apache.org <de...@curator.apache.org>>
>> <de...@curator.apache.org>
>> Date: August 1, 2014 at 2:32:07 PM
>> To: dev@curator.apache.org <de...@curator.apache.org>>
>> <de...@curator.apache.org>
>> Subject: Exception throwing
>>
>> I'd like to revisit the discussion around always throwing Exception in
>> the
>> API. There were two JIRA issues - CURATOR-135 and CURATOR-29 - that touch
>> on this subject, but I think there is a good conversation to be had.
>>
>> I understand the suggestions that if an exception is thrown, we are in a
>> bad state, regardless of the type of exception. However, throwing
>> Exception
>> comes with some unfortunate Java baggage...
>>
>> By declaring thrown Exception, we force consumers to also catch
>> RuntimeExceptions instead of letting them propagate as they normally
>> would.
>> In some cases, the calling code may need to attempt roll-back semantics,
>> or
>> retry outside of what Curator provides, or something else that we haven't
>> thought of.
>>
>> I'd like to propose replacing much of the thrown Exception methods with
>> CuratorException. This will still carry the connotation that it doesn't
>> matter what kind of exception we encounter, they're all bad. It will also
>> be backwards compatible with the previous API, since users will still be
>> able to catch Exception in their calling code. And it has the advantage of
>> separating checked exceptions from unchecked ones, so that users don't
>> unintentionally catch something unrelated.
>>
>> Thoughts?
>>
>> I tried looking for more details behind the design decision to always
>> throw
>> Exception, but wasn't able to find them. If they're already documented,
>> I'd
>> love to be pointed at the wiki or site, or a mailing list thread will do
>> in
>> a pinch.
>>
>> Thanks,
>> Mike
>>
>>
>
Re: Exception throwing
Posted by Jordan Zimmerman <jo...@jordanzimmerman.com>.
Like I said, if I could go back, I’d get rid of checked exceptions entirely.
-JZ
From: Mike Drob <ma...@cloudera.com>
Reply: Mike Drob <ma...@cloudera.com>>
Date: August 1, 2014 at 2:54:47 PM
To: Jordan Zimmerman <jo...@jordanzimmerman.com>>
Cc: dev@curator.apache.org <de...@curator.apache.org>>
Subject: Re: Exception throwing
So why declare that we throw exceptions instead of just throwing everything as a RuntimeException (or subclass thereof)?
On Fri, Aug 1, 2014 at 2:49 PM, Jordan Zimmerman <jo...@jordanzimmerman.com> wrote:
-1 (binding)
If I could go back I’d remove all checked exceptions entirely. I don’t think there’s an objective answer here - it comes down to personal preference, etc. I don’t see much value in touching nearly every file in the library in order to do this. We’ve had maybe 2 or 3 requests in the many years that Curator has exists. This suggests that the overwhelming majority accept the current exception semantics. If you can point to an actual bug that this causes that would be helpful.
-Jordan
From: Mike Drob <ma...@cloudera.com>
Reply: dev@curator.apache.org <de...@curator.apache.org>>
Date: August 1, 2014 at 2:32:07 PM
To: dev@curator.apache.org <de...@curator.apache.org>>
Subject: Exception throwing
I'd like to revisit the discussion around always throwing Exception in the
API. There were two JIRA issues - CURATOR-135 and CURATOR-29 - that touch
on this subject, but I think there is a good conversation to be had.
I understand the suggestions that if an exception is thrown, we are in a
bad state, regardless of the type of exception. However, throwing Exception
comes with some unfortunate Java baggage...
By declaring thrown Exception, we force consumers to also catch
RuntimeExceptions instead of letting them propagate as they normally would.
In some cases, the calling code may need to attempt roll-back semantics, or
retry outside of what Curator provides, or something else that we haven't
thought of.
I'd like to propose replacing much of the thrown Exception methods with
CuratorException. This will still carry the connotation that it doesn't
matter what kind of exception we encounter, they're all bad. It will also
be backwards compatible with the previous API, since users will still be
able to catch Exception in their calling code. And it has the advantage of
separating checked exceptions from unchecked ones, so that users don't
unintentionally catch something unrelated.
Thoughts?
I tried looking for more details behind the design decision to always throw
Exception, but wasn't able to find them. If they're already documented, I'd
love to be pointed at the wiki or site, or a mailing list thread will do in
a pinch.
Thanks,
Mike
Re: Exception throwing
Posted by Mike Drob <ma...@cloudera.com>.
So why declare that we throw exceptions instead of just throwing everything
as a RuntimeException (or subclass thereof)?
On Fri, Aug 1, 2014 at 2:49 PM, Jordan Zimmerman <jordan@jordanzimmerman.com
> wrote:
> -1 (binding)
>
> If I could go back I’d remove all checked exceptions entirely. I don’t
> think there’s an objective answer here - it comes down to personal
> preference, etc. I don’t see much value in touching nearly every file in
> the library in order to do this. We’ve had maybe 2 or 3 requests in the
> many years that Curator has exists. This suggests that the overwhelming
> majority accept the current exception semantics. If you can point to an
> actual bug that this causes that would be helpful.
>
> -Jordan
>
> From: Mike Drob <ma...@cloudera.com> <ma...@cloudera.com>
> Reply: dev@curator.apache.org <de...@curator.apache.org>>
> <de...@curator.apache.org>
> Date: August 1, 2014 at 2:32:07 PM
> To: dev@curator.apache.org <de...@curator.apache.org>>
> <de...@curator.apache.org>
> Subject: Exception throwing
>
> I'd like to revisit the discussion around always throwing Exception in the
> API. There were two JIRA issues - CURATOR-135 and CURATOR-29 - that touch
> on this subject, but I think there is a good conversation to be had.
>
> I understand the suggestions that if an exception is thrown, we are in a
> bad state, regardless of the type of exception. However, throwing
> Exception
> comes with some unfortunate Java baggage...
>
> By declaring thrown Exception, we force consumers to also catch
> RuntimeExceptions instead of letting them propagate as they normally
> would.
> In some cases, the calling code may need to attempt roll-back semantics,
> or
> retry outside of what Curator provides, or something else that we haven't
> thought of.
>
> I'd like to propose replacing much of the thrown Exception methods with
> CuratorException. This will still carry the connotation that it doesn't
> matter what kind of exception we encounter, they're all bad. It will also
> be backwards compatible with the previous API, since users will still be
> able to catch Exception in their calling code. And it has the advantage of
> separating checked exceptions from unchecked ones, so that users don't
> unintentionally catch something unrelated.
>
> Thoughts?
>
> I tried looking for more details behind the design decision to always
> throw
> Exception, but wasn't able to find them. If they're already documented,
> I'd
> love to be pointed at the wiki or site, or a mailing list thread will do
> in
> a pinch.
>
> Thanks,
> Mike
>
>
Re: Exception throwing
Posted by Jordan Zimmerman <jo...@jordanzimmerman.com>.
-1 (binding)
If I could go back I’d remove all checked exceptions entirely. I don’t think there’s an objective answer here - it comes down to personal preference, etc. I don’t see much value in touching nearly every file in the library in order to do this. We’ve had maybe 2 or 3 requests in the many years that Curator has exists. This suggests that the overwhelming majority accept the current exception semantics. If you can point to an actual bug that this causes that would be helpful.
-Jordan
From: Mike Drob <ma...@cloudera.com>
Reply: dev@curator.apache.org <de...@curator.apache.org>>
Date: August 1, 2014 at 2:32:07 PM
To: dev@curator.apache.org <de...@curator.apache.org>>
Subject: Exception throwing
I'd like to revisit the discussion around always throwing Exception in the
API. There were two JIRA issues - CURATOR-135 and CURATOR-29 - that touch
on this subject, but I think there is a good conversation to be had.
I understand the suggestions that if an exception is thrown, we are in a
bad state, regardless of the type of exception. However, throwing Exception
comes with some unfortunate Java baggage...
By declaring thrown Exception, we force consumers to also catch
RuntimeExceptions instead of letting them propagate as they normally would.
In some cases, the calling code may need to attempt roll-back semantics, or
retry outside of what Curator provides, or something else that we haven't
thought of.
I'd like to propose replacing much of the thrown Exception methods with
CuratorException. This will still carry the connotation that it doesn't
matter what kind of exception we encounter, they're all bad. It will also
be backwards compatible with the previous API, since users will still be
able to catch Exception in their calling code. And it has the advantage of
separating checked exceptions from unchecked ones, so that users don't
unintentionally catch something unrelated.
Thoughts?
I tried looking for more details behind the design decision to always throw
Exception, but wasn't able to find them. If they're already documented, I'd
love to be pointed at the wiki or site, or a mailing list thread will do in
a pinch.
Thanks,
Mike