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

Aaron Fabbri commented on HADOOP-13449:
---------------------------------------

Thanks for your hard work on this [~liuml07]!

{code}
-    <aws-java-sdk.version>1.10.6</aws-java-sdk.version>
+    <aws-java-sdk.version>1.11.0</aws-java-sdk.version>
{code}
We should apply this to trunk first, separately, and merge back to feature 
branch, as [~steve_l] suggested.  Let me know if you want help w/ that.

{code}
+  @Test
+  public void testDescendantsIterator() throws IOException {
{code}
 
Thanks for the extra test code!  Should we move this to MetadataStoreTestBase?  
That is, should this test run for any MetadataStore?

{code}
  
-  private void verifyBasicFileStatus(PathMetadata meta) {
-    FileStatus status = meta.getFileStatus();
+  void verifyFileStatus(FileStatus status, long size) {
     assertFalse("Not a dir", status.isDirectory());
-    assertEquals("Replication value", REPLICATION, status.getReplication());
-    assertEquals("Access time", accessTime, status.getAccessTime());
     assertEquals("Mod time", modTime, status.getModificationTime());
+    assertEquals("File size", size, status.getLen());
     assertEquals("Block size", BLOCK_SIZE, status.getBlockSize());
-    assertEquals("Owner", OWNER, status.getOwner());
-    assertEquals("Group", GROUP, status.getGroup());
-    assertEquals("Permission", PERMISSION, status.getPermission());
   }
  
-  private FileStatus makeDirStatus(String pathStr) {
+  private FileStatus makeDirStatus(String pathStr)
+      throws IOException {
     return basicFileStatus(new Path(pathStr), 0, true);
   }
  
-  private void verifyDirStatus(PathMetadata meta) {
-    FileStatus status = meta.getFileStatus();
+  /**
+   * Verify the directory file status. Subclass may verify additional fields.
+   */
+  void verifyDirStatus(FileStatus status) {
     assertTrue("Is a dir", status.isDirectory());
     assertEquals("zero length", 0, status.getLen());
-    assertEquals("Replication value", REPLICATION, status.getReplication());
-    assertEquals("Access time", accessTime, status.getAccessTime());
-    assertEquals("Mod time", modTime, status.getModificationTime());
-    assertEquals("Owner", OWNER, status.getOwner());
-    assertEquals("Group", GROUP, status.getGroup());
-    assertEquals("Permission", PERMISSION, status.getPermission());
+  }
+
{code}

Glad you got this working.  I'd like to keep this test code though.  Can we 
extend the MetadataStoreTestBase instead of changing it?  I made some 
suggestions 
[here|https://issues.apache.org/jira/browse/HADOOP-13650?focusedCommentId=15637956&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15637956].

Similarly, the changes in {{TestLocalMetadataStore}} should not be needed.  
Also I think that code has changed in the latest version posted in 
HADOOP-13651, so those changes would conflict on merge.

{code}
-    public MetadataStore getMetadataStore() throws IOException {
-      LocalMetadataStore lms = new LocalMetadataStore();
-      return lms;
+    public MetadataStore getMs() {
{code}

This function rename will probably cause merge headaches for both of us as 
well.  

Other than that, looks good.  Have you ran any of the integration / contract 
tests with it yet?  I suppose not since you'll need HADOOP-13651 first.


> S3Guard: Implement DynamoDBMetadataStore.
> -----------------------------------------
>
>                 Key: HADOOP-13449
>                 URL: https://issues.apache.org/jira/browse/HADOOP-13449
>             Project: Hadoop Common
>          Issue Type: Sub-task
>          Components: fs/s3
>            Reporter: Chris Nauroth
>            Assignee: Mingliang Liu
>         Attachments: HADOOP-13449-HADOOP-13345.000.patch, 
> HADOOP-13449-HADOOP-13345.001.patch, HADOOP-13449-HADOOP-13345.002.patch, 
> HADOOP-13449-HADOOP-13345.003.patch, HADOOP-13449-HADOOP-13345.004.patch
>
>
> Provide an implementation of the metadata store backed by DynamoDB.



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

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org

Reply via email to