HADOOP-18874: [ABFS] Adding Server returned request id in Exception Message thrown to caller. #6004
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
💔 -1 overall
This message was automatically generated. |
steveloughran
requested changes
Sep 1, 2023
There was a problem hiding this comment.
production code good.
for the tests, i want all new assertions to use assertJ with detailed assertions (.contains(), .startsWith(), .equals() etc). This is to have meaningful errors, rather than just "assert failed line 87" which is useless if it starts failing on jenkins runs. (*)
this would be best be matched by upgrading the existing ones to use it too, to avoid a mix
(*) it is predictable I will ask for this; save time and do it from the outset.
| // Check status message, status code, HTTP Request Type and URL. | ||
| Assert.assertEquals("Operation failed: \"The specified path does not exist.\"", errorFields[0].trim()); | ||
| Assert.assertEquals("404", errorFields[1].trim()); | ||
| Assert.assertEquals("HEAD", errorFields[2].trim()); | ||
| Assert.assertTrue(errorFields[3].trim().startsWith("http")); | ||
| Assert.assertTrue(errorFields[4].trim().startsWith("rId:")); |
There was a problem hiding this comment.
use assertj with a .startsWith() assertion.
| // Check status message, status code, HTTP Request Type and URL. | ||
| Assert.assertEquals("Operation failed: \"The specified path does not exist.\"", errorFields[0].trim()); | ||
| Assert.assertEquals("404", errorFields[1].trim()); | ||
| Assert.assertEquals("GET", errorFields[2].trim()); | ||
| Assert.assertTrue(errorFields[3].trim().startsWith("http")); | ||
| Assert.assertTrue(errorFields[4].trim().startsWith("rId:")); |
There was a problem hiding this comment.
use assertj with a .startsWith() assertion.
| Assert.assertTrue(errorFields[5].contains("RequestId") | ||
| && errorFields[5].contains("Time")); | ||
| Assert.assertEquals("PathNotFound", errorFields[5].trim()); | ||
| Assert.assertTrue(errorFields[6].contains("RequestId") |
There was a problem hiding this comment.
use assertj with a .contains() assertion.
anujmodi2021
force-pushed
the
HADOOP-18874-activityId
branch
from
77e2bb8
to
e586bf9
Compare
|
🎊 +1 overall
This message was automatically generated. |
|
Thanks for the review @steveloughran |
anujmodi2021
force-pushed
the
HADOOP-18874-activityId
branch
from
e586bf9
to
6bb403d
Compare
|
Recent Test Run :::: AGGREGATED TEST RESULT :::: HNS-OAuth[INFO] Results: HNS-SharedKey[INFO] Results: NonHNS-SharedKey[INFO] Results: AppendBlob-HNS-OAuth[INFO] Results: Time taken: 27 mins 20 secs. |
|
🎊 +1 overall
This message was automatically generated. |
anujmodi2021
added a commit
to anujmodi2021/hadoop
that referenced
this pull request
Nov 21, 2023
…wn to the caller. (apache#6004) Contributed by Anuj Modi
jiajunmao
pushed a commit
to jiajunmao/hadoop-MLEC
that referenced
this pull request
Feb 6, 2024
…wn to the caller. (apache#6004) Contributed by Anuj Modi
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Jira Ticket: https://issues.apache.org/jira/browse/HADOOP-18874
Description of PR
Each request made to Azure server has its unique ActivityId (rid) which is returned in response of the request whether is succeed or fails.
When a HDFS call fails due to an error from Azure service, An ABFSRestOperationException is throws to the caller. This task is to add a server returned activity id (rid) in the exception message which can be used to investigate the failure on service side.
How was this patch tested?
For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?