[jira] [Comment Edited] (HADOOP-12533) Introduce FileNotFoundException in WASB for read and seek API

["Chris Nauroth (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/HADOOP-12533?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14983560#comment-14983560 ] 

Chris Nauroth edited comment on HADOOP-12533 at 10/30/15 11:14 PM:
-------------------------------------------------------------------

[~dchickabasapa], thank you for the patch.

{code}
  public static final String AZURE_STORAGE_BLOBNOTFOUND_ERRORCODE = "BlobNotFound";
  public static final String AZURE_STORAGE_RESOURCENOTFOUND_ERRORCODE = "RESOURCE_NOT_FOUND";
{code}

Instead of repeating these constants within hadoop-azure, do you want to use {{com.microsoft.azure.storage#BLOB_NOT_FOUND}} and {{#RESOURCE_NOT_FOUND}}?  (BTW, is the second one really meant to have a value of "ResourceNotFound"?  That's how it's defined in the SDK.

{code}
            throw new FileNotFoundException();
{code}

Please pass a descriptive string to help with troubleshooting any time you call an exception constructor.  For a {{FileNotFoundException}}, including the file path is very helpful.

The exception unwinding logic is repeated multiple times.  Can that be refactored into a shared helper method?

bq. Since the files are deleted and the correctness of the test depends on getting the exception, I didn't close the streams.

Please ensure all streams get closed anyway at the end of the test.  If you need the stream to be left open while doing some other logic and running assertions, then that's fine, but proper cleanup at the end of a test run is still good practice.

bq. I think adding the AbstractFSContract suite of tests can be done as improvement.

This has come up as feedback on multiple code reviews.  The contract tests are a great way to catch early if a {{FileSystem}} implementation is not satisfying the semantics that Hadoop expects.  I appreciate controlling the scope of this patch, but I also see this as a deferred maintenance problem, ultimately causing more churn as patches come in piecemeal.

I've filed HADOOP-12535 to track work on the contract tests.  Dushyanth, I tentatively assigned this to you.  If this is not something that you're going to work on, please unassign it, so that someone else interested in the community can pick it up.


was (Author: cnauroth):
[~onpduo], thank you for the patch.

{code}
  public static final String AZURE_STORAGE_BLOBNOTFOUND_ERRORCODE = "BlobNotFound";
  public static final String AZURE_STORAGE_RESOURCENOTFOUND_ERRORCODE = "RESOURCE_NOT_FOUND";
{code}

Instead of repeating these constants within hadoop-azure, do you want to use {{com.microsoft.azure.storage#BLOB_NOT_FOUND}} and {{#RESOURCE_NOT_FOUND}}?  (BTW, is the second one really meant to have a value of "ResourceNotFound"?  That's how it's defined in the SDK.

{code}
            throw new FileNotFoundException();
{code}

Please pass a descriptive string to help with troubleshooting any time you call an exception constructor.  For a {{FileNotFoundException}}, including the file path is very helpful.

The exception unwinding logic is repeated multiple times.  Can that be refactored into a shared helper method?

bq. Since the files are deleted and the correctness of the test depends on getting the exception, I didn't close the streams.

Please ensure all streams get closed anyway at the end of the test.  If you need the stream to be left open while doing some other logic and running assertions, then that's fine, but proper cleanup at the end of a test run is still good practice.

bq. I think adding the AbstractFSContract suite of tests can be done as improvement.

This has come up as feedback on multiple code reviews.  The contract tests are a great way to catch early if a {{FileSystem}} implementation is not satisfying the semantics that Hadoop expects.  I appreciate controlling the scope of this patch, but I also see this as a deferred maintenance problem, ultimately causing more churn as patches come in piecemeal.

I've filed HADOOP-12535 to track work on the contract tests.  Duo, I tentatively assigned this to you.  If this is not something that you're going to work on, please unassign it, so that someone else interested in the community can pick it up.

> Introduce FileNotFoundException in WASB for read and seek API
> -------------------------------------------------------------
>
>                 Key: HADOOP-12533
>                 URL: https://issues.apache.org/jira/browse/HADOOP-12533
>             Project: Hadoop Common
>          Issue Type: Bug
>          Components: tools
>    Affects Versions: 2.8.0
>            Reporter: Dushyanth
>            Assignee: Dushyanth
>             Fix For: 2.8.0
>
>         Attachments: HADOOP-12533.001.patch
>
>
> Currently WASB throws a IOException in read and seek API for both Block and Page blobs for scenarios where the backing blobs do not exists. This creates problems for applications like HBase which expect a FileNotFoundException in these scenarios. 
> The fix for the problem is to check if the exceptions from Azure storage is because for Blob not found and throw FileNotFound exception if that is the case.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)