Re: [PR] NIFI-13265 Removed the instantiation of Object arrays for arguments in ComponentLog log and org.slf4j.Logger statements. [nifi]

2024-05-30 Thread via GitHub


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]

2024-05-30 Thread via GitHub


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]

2024-05-30 Thread via GitHub


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]

2024-05-30 Thread via GitHub


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]

2024-05-30 Thread via GitHub


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]

2024-05-30 Thread via GitHub


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]

2024-05-30 Thread via GitHub


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]

2024-05-30 Thread via GitHub


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]

2024-05-29 Thread via GitHub


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