Conversation
anmolanmol1234
requested review from
anujmodi2021,
saxenapranav,
snvijaya and
sreeb-msft
saxenapranav
left a comment
saxenapranav
left a comment
There was a problem hiding this comment.
Looks good with code change. One question:
When at https://github.com/ABFSDriver/AbfsHadoop/blob/ABFS_3.3.2_dev/hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsClient.java#L899, when we retry without 100-continue header, agent will still show as 100-continue header. Should we change agent dynamically for that?
saxenapranav
left a comment
saxenapranav
left a comment
There was a problem hiding this comment.
Thanks for taking suggestions. Great code!
| */ | ||
| public class AbfsClient implements Closeable { | ||
| public static final Logger LOG = LoggerFactory.getLogger(AbfsClient.class); | ||
| public static final String HUNDRED_CONTINUE_USER_AGENT = " 100-continue;"; |
There was a problem hiding this comment.
Can we replace it with something like this: HUNDRED_CONTINUE_USER_AGENT = SINGLE_WHITE_SPACE + "100-continue" + SEMI_COLON;
This way it would assert that these characters are intentionally added.
There was a problem hiding this comment.
May be we can define a constant for "100-continue" as well, I can see its being used at multiple places
There was a problem hiding this comment.
We already have a constant for 100-continue, will take this up
|
|
||
| if (abfsConfiguration.isExpectHeaderEnabled()) { | ||
| sb.append(SINGLE_WHITE_SPACE); | ||
| sb.append("100-continue"); |
There was a problem hiding this comment.
Here also we can use the constant
3 participants
Adding 100 continue in userAgent String if enabled in AbfsConfiguration and dynamically removing it if retry is without the header enabled.