exceptionfactory commented on a change in pull request #5098:
URL: https://github.com/apache/nifi/pull/5098#discussion_r645581206
##########
File path:
nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/processors/azure/storage/PutAzureDataLakeStorage.java
##########
@@ -192,4 +211,29 @@ static void uploadContent(DataLakeFileClient fileClient,
InputStream in, long le
fileClient.flush(length);
}
+
+ private DataLakeFileClient renameFile(String fileName, String
directoryPath, DataLakeFileClient fileClient, boolean overwrite) {
Review comment:
Recommend adding `final` to these parameters.
##########
File path:
nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/processors/azure/storage/ListAzureDataLakeStorage.java
##########
@@ -129,6 +131,16 @@
.expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
.build();
+ public static final PropertyDescriptor TEMP_FILE_PREFIX = new
PropertyDescriptor.Builder()
+ .name("azure-temp-file-prefix")
+ .displayName("Temp File Prefix for Azure")
Review comment:
The concept seems straightforward, but given the behavior, would it make
sense to rename this to something more general? Without reading the
description, at first it sounds like the property will be used to create
temporary files as opposed to filtering them. What do you think about renaming
this property and associated variables to something like `Filter File Prefix`?
Taking the concept one step further, would it make sense to change this to a
Regular Expression pattern instead of a prefix?
In either case the `for Azure` portion of the property display name seems
unnecessary since this component is already specific to Azure.
##########
File path:
nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/processors/azure/storage/ListAzureDataLakeStorage.java
##########
@@ -226,9 +239,11 @@ protected String getPath(ProcessContext context) {
options.setRecursive(recurseSubdirectories);
Pattern baseDirectoryPattern = Pattern.compile("^" + baseDirectory
+ "/?");
+ String tempFilePrefix =
context.getProperty(TEMP_FILE_PREFIX).evaluateAttributeExpressions().getValue();
Review comment:
Recommend adding `final` to this variable declaration.
##########
File path:
nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/processors/azure/storage/PutAzureDataLakeStorage.java
##########
@@ -192,4 +211,29 @@ static void uploadContent(DataLakeFileClient fileClient,
InputStream in, long le
fileClient.flush(length);
}
+
+ private DataLakeFileClient renameFile(String fileName, String
directoryPath, DataLakeFileClient fileClient, boolean overwrite) {
+ try {
+ final DataLakeRequestConditions destinationCondition = new
DataLakeRequestConditions();
+ if (!overwrite) {
+ destinationCondition.setIfNoneMatch("*");
+ }
+ String destinationPath = StringUtils.isNotEmpty(directoryPath)
+ ? directoryPath + "/" + fileName
+ : fileName;
+ return fileClient.renameWithResponse(null, destinationPath, null,
destinationCondition, null, null).getValue();
+ } catch (DataLakeStorageException dataLakeStorageException) {
+ getLogger().error("Error while renaming temp file on Azure Data
Lake Storage", dataLakeStorageException);
+ removeTempFile(fileClient);
+ throw dataLakeStorageException;
+ }
+ }
+
+ private void removeTempFile(DataLakeFileClient fileClient) {
+ try {
+ fileClient.delete();
+ } catch (Exception e) {
+ getLogger().error("Error while removing temp file on Azure Data
Lake Storage", e);
Review comment:
Are there any additional details worth including from
`DataLakeFileClient` in the error message, such as the file name?
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
