Changes done for adding formats to the Azure Blob Storage plugin#163
Changes done for adding formats to the Azure Blob Storage plugin#163vikasrathee-cs wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the Azure Blob Store batch source to support Azure Data Lake Storage Gen2 (ABFS) alongside WASB, introducing new authentication methods such as Service Principal and Managed Identity. The implementation is updated to extend AbstractFileSource instead of AbstractFileBatchSource, with corresponding updates to documentation, dependencies, and UI widgets. The review feedback highlights critical safety improvements, specifically pointing out potential NullPointerException risks when validating or parsing null path and account values, as well as identifying obsolete UI widgets that should be removed.
ea61df6 to
726e15a
Compare
| <properties> | ||
| <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> | ||
| <cdap.version>6.11.0</cdap.version> | ||
| <cdap.version>6.10.0</cdap.version> |
|
|
||
| @Override | ||
| protected void recordLineage(LineageRecorder lineageRecorder, List<String> outputFields) { | ||
| lineageRecorder.recordRead("Read", "Read from Azure Blob Storage.", outputFields); |
There was a problem hiding this comment.
Nit :Change to "Read from Azure Storage" .
So, it covers gen 2 as well.
| .withConfigProperty(PATH); | ||
|
|
||
| boolean isWasb = !containsMacro(NAME_PATH) && | ||
| path != null && (path.startsWith("wasb://") || path.startsWith("wasbs://")); |
There was a problem hiding this comment.
Please create private static final String constants at top for wasb://, wasbs:// etc... and replace these.
Applicable for all below.
| } | ||
|
|
||
| String authMethod = config.authenticationMethod; | ||
| String account = config.account; |
| <cdap.plugin.version>2.13.0</cdap.plugin.version> | ||
| <cdap.plugin.version>2.12.1</cdap.plugin.version> | ||
| <jackson.databind.version>2.13.4.2</jackson.databind.version> | ||
| <guava.version>27.0-jre</guava.version> |
There was a problem hiding this comment.
is this needed ?
If so can we target the guava version we are going to use in CDAP in guava upgrade case ?
Changes done for adding formats to the Azure Blob Storage plugin