You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2022/08/05 19:05:01 UTC
[GitHub] [accumulo] dlmarion opened a new issue, #2853: ServerAmpleImpl should throw more narrow exceptions
dlmarion opened a new issue, #2853:
URL: https://github.com/apache/accumulo/issues/2853
Almost every method in ServerAmpleImpl throws RuntimeException. Modify to throw more narrow exceptions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] cshannon commented on issue #2853: ServerAmpleImpl should throw more narrow exceptions
Posted by GitBox <gi...@apache.org>.
cshannon commented on issue #2853:
URL: https://github.com/apache/accumulo/issues/2853#issuecomment-1207245395
@dlmarion - As I am sure you are aware, using more narrow exceptions would add the checked exceptions to the methods of the interface [Ample.java](https://github.com/apache/accumulo/blob/main/core/src/main/java/org/apache/accumulo/core/metadata/schema/Ample.java) which would cause a lot of cascading effects and require changes in several other places of the code to now handle all the checked exceptions (not to mention it would be a breaking change but that is probably ok since it's server side and not in the client). I assume that's fine and that is the intent here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] dlmarion commented on issue #2853: ServerAmpleImpl should throw more narrow exceptions
Posted by GitBox <gi...@apache.org>.
dlmarion commented on issue #2853:
URL: https://github.com/apache/accumulo/issues/2853#issuecomment-1208326538
See https://github.com/apache/accumulo/pull/2665/files#r938956933 and https://github.com/apache/accumulo/pull/2665/files#r938957079
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] cshannon commented on issue #2853: ServerAmpleImpl should throw more narrow exceptions
Posted by GitBox <gi...@apache.org>.
cshannon commented on issue #2853:
URL: https://github.com/apache/accumulo/issues/2853#issuecomment-1207245926
Looking at this more the other issue is that there are other implementations of Ample that wouldn't necessarily throw the same checked exceptions (in fact the default methods in the interface throw unsupported exceptions) so adding checked exceptions to the interface doesn't seem like a good idea. Maybe we need to create or just use some more existing narrow runtime exceptions instead to prevent having to add checked exceptions to the interface?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] dlmarion commented on issue #2853: ServerAmpleImpl should throw more narrow exceptions
Posted by GitBox <gi...@apache.org>.
dlmarion commented on issue #2853:
URL: https://github.com/apache/accumulo/issues/2853#issuecomment-1208006123
> As I am sure you are aware, using more narrow exceptions would add the checked exceptions to the methods of the interface [Ample.java](https://github.com/apache/accumulo/blob/main/core/src/main/java/org/apache/accumulo/core/metadata/schema/Ample.java) which would cause a lot of cascading effects and require changes in several other places of the code to now handle all the checked exceptions (not to mention it would be a breaking change but that is probably ok since it's server side and not in the client). I assume that's fine and that is the intent here?
@cshannon - I will let @ctubbsii respond to that, this issue was spawned from a comment he made on #2665 .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on issue #2853: ServerAmpleImpl should throw more narrow exceptions
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #2853:
URL: https://github.com/apache/accumulo/issues/2853#issuecomment-1208319886
The comments were very specific to the code I had commented on. I know we have overly broad exception handling and throwing. The original comments were to avoid introducing new such occurrences... not to rewrite all the existing code to fix it everywhere.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] dlmarion closed issue #2853: ServerAmpleImpl should throw more narrow exceptions
Posted by GitBox <gi...@apache.org>.
dlmarion closed issue #2853: ServerAmpleImpl should throw more narrow exceptions
URL: https://github.com/apache/accumulo/issues/2853
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on issue #2853: ServerAmpleImpl should throw more narrow exceptions
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #2853:
URL: https://github.com/apache/accumulo/issues/2853#issuecomment-1208332695
> See https://github.com/apache/accumulo/pull/2665/files#r938956933 and https://github.com/apache/accumulo/pull/2665/files#r938957079
Links to comments should take the form `https://github.com/apache/accumulo/pull/2665#discussion_r938955552`. I'm not sure what your links are intended to take the user to, but they don't seem to work for me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org