Re: [PR] NIFI-13265 Removed the instantiation of Object arrays for arguments in ComponentLog log and org.slf4j.Logger statements. [nifi]
exceptionfactory commented on PR #8896: URL: https://github.com/apache/nifi/pull/8896#issuecomment-2140949450 > @exceptionfactory Can I just then modify NIFI-13323 to include other places besides the HashAttribute and PostHttp Processors? Sure, that sounds like a good plan. -- 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-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-13265 Removed the instantiation of Object arrays for arguments in ComponentLog log and org.slf4j.Logger statements. [nifi]
dan-s1 commented on PR #8896: URL: https://github.com/apache/nifi/pull/8896#issuecomment-2140948368 @exceptionfactory Can I just then modify NIFI-13323 to include other places besides the HashAttribute and PostHttp Processors? -- 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-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-13265 Removed the instantiation of Object arrays for arguments in ComponentLog log and org.slf4j.Logger statements. [nifi]
exceptionfactory commented on PR #8896: URL: https://github.com/apache/nifi/pull/8896#issuecomment-2140945458 @dan-s1 This commit does not apply cleanly to the support branch given the number of other changes, so if you are interested in having it applied, it would require a separate pull request. I also recommend handling it in a different Jira issue, since there are likely a number of other classes, now removed from main, where this would otherwise apply. -- 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-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-13265 Removed the instantiation of Object arrays for arguments in ComponentLog log and org.slf4j.Logger statements. [nifi]
exceptionfactory closed pull request #8896: NIFI-13265 Removed the instantiation of Object arrays for arguments in ComponentLog log and org.slf4j.Logger statements. URL: https://github.com/apache/nifi/pull/8896 -- 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-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-13265 Removed the instantiation of Object arrays for arguments in ComponentLog log and org.slf4j.Logger statements. [nifi]
exceptionfactory commented on code in PR #8896: URL: https://github.com/apache/nifi/pull/8896#discussion_r1621169344 ## nifi-extension-bundles/nifi-aws-bundle/nifi-aws-processors/src/main/java/org/apache/nifi/processors/aws/sqs/GetSQS.java: ## @@ -223,8 +223,7 @@ private void deleteMessages(final SqsClient client, final String queueUrl, final try { client.deleteMessageBatch(deleteRequest); } catch (final Exception e) { -getLogger().error("Received {} messages from Amazon SQS but failed to delete the messages; these messages" -+ " may be duplicated. Reason for deletion failure: {}", new Object[]{messages.size(), e}); +getLogger().error("Received {} messages from Amazon SQS but failed to delete the messages; these messages may be duplicated. Reason for deletion failure: ", messages.size(), e); Review Comment: ```suggestion getLogger().error("Received {} messages from Amazon SQS but failed to delete the messages; these messages may be duplicated", messages.size(), e); ``` ## nifi-extension-bundles/nifi-aws-bundle/nifi-aws-processors/src/main/java/org/apache/nifi/processors/aws/s3/PutS3Object.java: ## @@ -925,12 +908,10 @@ protected MultipartUploadListing getS3AgeoffListAndAgeoffLocalState(final Proces getLogger().warn("AccessDenied checking S3 Multipart Upload list for {}: {} " + "** The configured user does not have the s3:ListBucketMultipartUploads permission " + "for this bucket, S3 ageoff cannot occur without this permission. Next ageoff check " + -"time is being advanced by interval to prevent checking on every upload **", -new Object[]{bucket, e.getMessage()}); +"time is being advanced by interval to prevent checking on every upload **", bucket, e.getMessage()); lastS3AgeOff.set(System.currentTimeMillis()); } else { -getLogger().error("Error checking S3 Multipart Upload list for {}: {}", -new Object[]{bucket, e.getMessage()}); +getLogger().error("Error checking S3 Multipart Upload list for {}:", bucket, e); Review Comment: ```suggestion getLogger().error("Error checking S3 Multipart Upload list for {}", bucket, e); ``` ## nifi-extension-bundles/nifi-hadoop-bundle/nifi-hdfs-processors/src/main/java/org/apache/nifi/processors/hadoop/FetchHDFS.java: ## @@ -181,7 +181,7 @@ public void onTrigger(final ProcessContext context, final ProcessSession session session.getProvenanceReporter().fetch(outgoingFlowFile, qualifiedPath.toString(), stopWatch.getDuration(TimeUnit.MILLISECONDS)); session.transfer(outgoingFlowFile, getSuccessRelationship()); } catch (final FileNotFoundException | AccessControlException e) { -getLogger().error("Failed to retrieve content from {} for {} due to {}; routing to failure", new Object[]{qualifiedPath, outgoingFlowFile, e}); +getLogger().error("Routing to failure since failed to retrieve content from {} for {}", qualifiedPath, outgoingFlowFile, e); Review Comment: ```suggestion getLogger().error("Failed to retrieve content from {} for {}", qualifiedPath, outgoingFlowFile, e); ``` ## nifi-extension-bundles/nifi-hadoop-bundle/nifi-hdfs-processors/src/main/java/org/apache/nifi/processors/hadoop/inotify/GetHDFSEvents.java: ## @@ -223,15 +223,15 @@ public void process(OutputStream out) throws IOException { lastTxId = eventBatch.getTxid(); } } catch (IOException | InterruptedException e) { -getLogger().error("Unable to get notification information: {}", new Object[]{e}); +getLogger().error("Unable to get notification information:", e); Review Comment: ```suggestion getLogger().error("Unable to get notification information", e); ``` ## nifi-extension-bundles/nifi-hadoop-bundle/nifi-hdfs-processors/src/main/java/org/apache/nifi/processors/hadoop/KeyValueReader.java: ## @@ -68,7 +68,7 @@ public Set readSequenceFile(Path file, Configuration configuration, Fi final KeyValueWriterCallback callback = new KeyValueWriterCallback(reader); final String inputfileName = file.getName() + "." + System.nanoTime() + "."; int counter = 0; -LOG.debug("Read from SequenceFile: {} ", new Object[]{file}); +LOG.debug("Read from SequenceFile: {} ", file); Review Comment: ```suggestion LOG.debug("Read from SequenceFile: {}", file); ``` ## nifi-extension-bundles/nifi-hbase-bundle/nifi-hbase-processors/src/main/java/org/apache/nifi/hbase/AbstractPutHBase.j
Re: [PR] NIFI-13265 Removed the instantiation of Object arrays for arguments in ComponentLog log and org.slf4j.Logger statements. [nifi]
exceptionfactory commented on PR #8896: URL: https://github.com/apache/nifi/pull/8896#issuecomment-213928 > @exceptionfactory Thanks for the help on that! Looking at the javadoc on the method `isCommandFailed` > > ``` > /** > * On some environment, the test command immediately fail with an IOException > * because of the native UnixProcess.init method implementation difference. > * > * @return true, if the command fails > */ > ``` > > it would seems to indicate that the JUnit 5 `@EnabledOnOs` annotation should be used to run those unit tests this method is used in to only run on a Unix environment. Yes, that is just one of several issues with the test class as it stands. There are System.out.println() statements that should be removed, System properties that should be removed, and other issues. It would be worth rewriting from scratch, with the goal of modernizing the implementation and providing better coverage of some scenarios. Not something for this Jira issue, but something to consider separately. -- 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-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-13265 Removed the instantiation of Object arrays for arguments in ComponentLog log and org.slf4j.Logger statements. [nifi]
dan-s1 commented on PR #8896: URL: https://github.com/apache/nifi/pull/8896#issuecomment-2139843287 @exceptionfactory Thanks for the help on that! Looking at the javadoc on the method `isCommandFailed` ``` /** * On some environment, the test command immediately fail with an IOException * because of the native UnixProcess.init method implementation difference. * * @return true, if the command fails */ ``` it would seems to indicate that the JUnit 5 `@EnabledOnOs` annotation should be used to run those unit tests this method is used in to only run on a Unix environment. -- 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-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-13265 Removed the instantiation of Object arrays for arguments in ComponentLog log and org.slf4j.Logger statements. [nifi]
exceptionfactory commented on PR #8896: URL: https://github.com/apache/nifi/pull/8896#issuecomment-2139613747 > @exceptionfactory Can you please advise on iif s there something I checked in that caused the following exceptions > > ``` > Error:TestExecuteProcess.testNotRedirectErrorStream:241 If redirect error stream is false, the output should be logged as a warning so that user can notice on bulletin. ==> expected: <1> but was: <0> > Error:TestExecuteProcess.testRedirectErrorStream:268 expected: <1> but was: <0> > ``` > > Those tests on my computer are working and they are working on the MacOS. What more can I do for this? @dan-s1 The `TestExecuteProcess` class has some design issues, but it appears that this particular issue could be related to the `isCommandFailed()` method in `TestExecuteProcess`. That method looks for an error message containing the words `due to` on line 308. Removing the `due to` portion of the string should align with the changes you have implemented. -- 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-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] NIFI-13265 Removed the instantiation of Object arrays for arguments in ComponentLog log and org.slf4j.Logger statements. [nifi]
dan-s1 commented on PR #8896: URL: https://github.com/apache/nifi/pull/8896#issuecomment-2138336598 @exceptionfactory Can you please advise on is there something I checked in that caused the following exceptions ``` Error:TestExecuteProcess.testNotRedirectErrorStream:241 If redirect error stream is false, the output should be logged as a warning so that user can notice on bulletin. ==> expected: <1> but was: <0> Error:TestExecuteProcess.testRedirectErrorStream:268 expected: <1> but was: <0> ``` Those tests on my computer are working and they are working on the MacOS. What more can I do for this? -- 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-unsubscr...@nifi.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org