steveloughran commented on code in PR #5621:
URL: https://github.com/apache/hadoop/pull/5621#discussion_r1186901863


##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/performance/ITestS3ADeleteCost.java:
##########
@@ -181,6 +184,8 @@ public void testDeleteFileInDir() throws Throwable {
 
   @Test
   public void testDirMarkersSubdir() throws Throwable {
+    LogCapturer logCapturer =

Review Comment:
   what are you trying to do here? I really don't like log capture for its 
brittleness and ugliness.
   
   Add something in TestHttpReferrerAuditHeader to actually examine the header.



##########
hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/auditing.md:
##########
@@ -221,22 +222,23 @@ 
https://audit.example.org/hadoop/1/op_rename/3c0d9b7e-2a63-43d9-a220-3c574d768ef
 Here are the fields which may be found in a request.
 If any of the field values were `null`, the field is omitted.
 
-| Name | Meaning | Example |
-|------|---------|---------|
-| `cm` | Command | `S3GuardTool$BucketInfo` |
-| `fs` | FileSystem ID | `af5943a9-b6f6-4eec-9c58-008982fc492a` |
-| `id` | Span ID | `3c0d9b7e-2a63-43d9-a220-3c574d768ef3-3` |
-| `ji` | Job ID  (S3A committer)| `(Generated by query engine)` |
-| `op` | Filesystem API call | `op_rename` |
-| `p1` | Path 1 of operation | `s3a://alice-london/path1` |
-| `p2` | Path 2 of operation | `s3a://alice-london/path2` |
-| `pr` | Principal | `alice` |
-| `ps` | Unique process UUID | `235865a0-d399-4696-9978-64568db1b51c` |
-| `rg` | GET request range | `100-200` |
-| `ta` | Task Attempt ID (S3A committer) | |
-| `t0` | Thread 0: thread span was created in | `100` |
-| `t1` | Thread 1: thread this operation was executed in | `200` |
-| `ts` | Timestamp (UTC epoch millis) | `1617116985923` |
+| Name | Meaning                                                               
                               | Example |

Review Comment:
   don't do the ide reformat as it changes the width of the table every time. 
just add the new row



##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/performance/ITestS3ARenameCost.java:
##########
@@ -124,6 +126,22 @@ public void testRenameFileToDifferentDirectory() throws 
Throwable {
         withWhenDeleting(OBJECT_DELETE_OBJECTS,
             directoriesInPath + 1));
 
+    if (isKeepingMarkers()) {
+      String output = logCapturer.getOutput();
+      String[] logs = output.split("\\n");
+      assertTrue("Num of files to delete should be 1",

Review Comment:
   consider my reviews to be an automatic -1 on any assert where assertJ does 
better, especially things like their .contains() predicate. save time by using 
assertJ from the outset.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to