You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2021/10/25 20:49:01 UTC
[GitHub] [nifi] exceptionfactory commented on a change in pull request #5478: NIFI-9328: Transfer cleanup and reuse added to FetchFileTransfer
exceptionfactory commented on a change in pull request #5478:
URL: https://github.com/apache/nifi/pull/5478#discussion_r735952746
##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/FetchFileTransfer.java
##########
@@ -261,18 +261,22 @@ public void onTrigger(final ProcessContext context, final ProcessSession session
flowFile = transfer.getRemoteFile(filename, flowFile, session);
} catch (final FileNotFoundException e) {
- closeConnection = false;
getLogger().log(levelFileNotFound, "Failed to fetch content for {} from filename {} on remote host {} because the file could not be found on the remote system; routing to {}",
- new Object[]{flowFile, filename, host, REL_NOT_FOUND.getName()});
+ flowFile, filename, host, REL_NOT_FOUND.getName());
session.transfer(session.penalize(flowFile), REL_NOT_FOUND);
session.getProvenanceReporter().route(flowFile, REL_NOT_FOUND);
+ if (transfer != null) {
Review comment:
The `null` check for `transfer` should not be necessary here or in the other catch block.
##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/FetchFileTransfer.java
##########
@@ -261,18 +261,22 @@ public void onTrigger(final ProcessContext context, final ProcessSession session
flowFile = transfer.getRemoteFile(filename, flowFile, session);
} catch (final FileNotFoundException e) {
- closeConnection = false;
getLogger().log(levelFileNotFound, "Failed to fetch content for {} from filename {} on remote host {} because the file could not be found on the remote system; routing to {}",
- new Object[]{flowFile, filename, host, REL_NOT_FOUND.getName()});
+ flowFile, filename, host, REL_NOT_FOUND.getName());
session.transfer(session.penalize(flowFile), REL_NOT_FOUND);
session.getProvenanceReporter().route(flowFile, REL_NOT_FOUND);
+ if (transfer != null) {
+ cleanupTransfer(transfer, closeConnection, transferQueue, host, port);
+ }
return;
} catch (final PermissionDeniedException e) {
- closeConnection = false;
getLogger().error("Failed to fetch content for {} from filename {} on remote host {} due to insufficient permissions; routing to {}",
- new Object[]{flowFile, filename, host, REL_PERMISSION_DENIED.getName()});
+ flowFile, filename, host, REL_PERMISSION_DENIED.getName());
session.transfer(session.penalize(flowFile), REL_PERMISSION_DENIED);
session.getProvenanceReporter().route(flowFile, REL_PERMISSION_DENIED);
+ if (transfer != null) {
+ cleanupTransfer(transfer, closeConnection, transferQueue, host, port);
+ }
return;
} catch (final ProcessException | IOException e) {
closeConnection = true;
Review comment:
This `closeConnection` variable can be removed here and from other places in this method.
##########
File path: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/FetchFileTransfer.java
##########
@@ -261,18 +261,22 @@ public void onTrigger(final ProcessContext context, final ProcessSession session
flowFile = transfer.getRemoteFile(filename, flowFile, session);
} catch (final FileNotFoundException e) {
- closeConnection = false;
getLogger().log(levelFileNotFound, "Failed to fetch content for {} from filename {} on remote host {} because the file could not be found on the remote system; routing to {}",
- new Object[]{flowFile, filename, host, REL_NOT_FOUND.getName()});
+ flowFile, filename, host, REL_NOT_FOUND.getName());
session.transfer(session.penalize(flowFile), REL_NOT_FOUND);
session.getProvenanceReporter().route(flowFile, REL_NOT_FOUND);
+ if (transfer != null) {
+ cleanupTransfer(transfer, closeConnection, transferQueue, host, port);
+ }
return;
} catch (final PermissionDeniedException e) {
- closeConnection = false;
getLogger().error("Failed to fetch content for {} from filename {} on remote host {} due to insufficient permissions; routing to {}",
- new Object[]{flowFile, filename, host, REL_PERMISSION_DENIED.getName()});
+ flowFile, filename, host, REL_PERMISSION_DENIED.getName());
session.transfer(session.penalize(flowFile), REL_PERMISSION_DENIED);
session.getProvenanceReporter().route(flowFile, REL_PERMISSION_DENIED);
+ if (transfer != null) {
+ cleanupTransfer(transfer, closeConnection, transferQueue, host, port);
+ }
return;
} catch (final ProcessException | IOException e) {
Review comment:
This catch block also needs to call `cleanupTransfer()`.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org