Wrapped continuation token with Exception when recursive acl call is … #15839
Conversation
ghost
added
the
...age-file-datalake/src/main/java/com/azure/storage/file/datalake/DataLakePathAsyncClient.java
Outdated
Show resolved
Hide resolved
| /** | ||
| * An exception thrown when an operation is interrupted and can be continued later on. | ||
| */ | ||
| public class DataLakeAclChangeFailedException extends RuntimeException { |
There was a problem hiding this comment.
Should we make it checked exception? That would be compiler enforced reminder to add proper error handling for this api (during design review this was a hot topic - how to write resilient code for this api). Unless there's a guidance (I couldn't find any) or limitation (i.e. reactor doesn't like checked exceptions)
There was a problem hiding this comment.
Like do you mean defining the method to say " throws DataLakeAclChangeFailedException"?
There was a problem hiding this comment.
It's a good idea but it also doesnt match with all the other APIs where we throw DataLakeStorageExceptoin for network errors
There was a problem hiding this comment.
throws and make it extend Exception. I'm not sure which one is more important - not standing out (which this API does anyway) or highlight that some error handling should be written to not miss ACL updates as this is SDK responsibility partially with current design. @alzimmermsft what do you think?
There was a problem hiding this comment.
Generally we have erred on using Runtime exceptions to prevent downstream code from requiring try/catch blocks everywhere. https://azure.github.io/azure-sdk/java_implementation.html#error-handling
There have been cases in the past where we do use checked exceptions, but those are generally to maintain exception types with a Java language interface.
@srnagar @JonathanGiles for a bit more input.
There was a problem hiding this comment.
Gotcha. So there is a guidance. Let's stick to it then.
There was a problem hiding this comment.
Agree with @alzimmermsft. We should be using appropriate derivatives of RuntimeException for most client library APIs to not force users to write try/catch blocks. Also, it is preferable to use one of the Java runtime exception types or exceptions defined in Azure core instead of creating new custom exceptions unless it is necessary to define a new type.
There was a problem hiding this comment.
I believe we were asked to create a new Exception type from guidance in .NET from Ted and Kryzsztof we are just following suit here as well. The reason we need a custom type is because we need to expose the continuation token to the user.
gapra-msft
requested review from
amishra-dev,
jaschrep-msft and
rickle-msft
as code owners
* Updated all service versions to STG74 (#14079) * Added code for get file range diff (#14140) * Added code for smb multi channel (#14180) * Added code to allow scheduling file expiry (#14319) * Added support for arrow output serialization (#14431) * Added support to read last access time (#14342) * Added support to lease shares (#14287) * Updated file ranges to getFileRangesDiff (#14839) * Added support for directory and delegation SAS (#14531) * Recursive acl (#14669) * Added missing error code (#14986) * Added tests to ensure support for 4TB file (#15179) Co-authored-by: Gauri Prasad <[email protected]> * Storage/file share error code (#15007) * Fixed simple renames and doc issues from 74 (#15297) Co-authored-by: Gauri Prasad <[email protected]> * Added back support for container undelete. (#15344) * Regenerated code to address APIView comments (#15341) * Minor changelog formatting issues * Formatting - new lines and unused imports * Fixed public API for file get range diff (#15562) * Modified recursive acl tests to be able to play in live mode (#15815) * Added support for live tests in the STG 74 branch (#15724) * Added code to return batch failures in results for recursive ACL (#15842) * Wrapped continuation token with Exception when recursive acl call is … (#15839) Co-authored-by: Gauri Prasad <[email protected]> Co-authored-by: Rick Ley <[email protected]>
…interrupted