NIFI-11995 Support multiple output formats for message headers in ConsumeAMQP#7652
NIFI-11995 Support multiple output formats for message headers in ConsumeAMQP#7652umarhussain15 wants to merge 5 commits intoapache:mainfrom
Conversation
nandorsoma
left a comment
There was a problem hiding this comment.
Thank you, @umarhussain15, for your contribution. I noticed that there are a bunch of unnecessary reformats. Could you revert the ones where you didn't change the logic before we proceed?
|
Thanks, @nandorsoma, I have updated the pull request. |
|
Thanks @umarhussain15. Unfortunately, we have work to do on this topic. We have a pretty minimal |
exceptionfactory
left a comment
There was a problem hiding this comment.
Thanks for your work on this feature @umarhussain15.
Do you have any additional feedback on this pull request @nandorsoma?
I noted one concerning about handling failures for JSON formatting.
...p-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/ConsumeAMQP.java
Outdated
Show resolved
Hide resolved
Co-authored-by: exceptionfactory <exceptionfactory@apache.org>
|
Hi @nandorsoma /@exceptionfactory, the ci-workflow failed for the MacOS build due to java heap space issue. Do I need to make some changes, or will you re-run the failed stage? |
exceptionfactory
left a comment
There was a problem hiding this comment.
Thanks for your patience on this pull request @umarhussain15. This looks close to completion, I noted a few implementation details and wording recommendations, and then it should be ready to go.
...p-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/ConsumeAMQP.java
Outdated
Show resolved
Hide resolved
...p-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/ConsumeAMQP.java
Outdated
Show resolved
Hide resolved
...p-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/ConsumeAMQP.java
Outdated
Show resolved
Hide resolved
...ndle/nifi-amqp-processors/src/test/java/org/apache/nifi/amqp/processors/ConsumeAMQPTest.java
Outdated
Show resolved
Hide resolved
...ndle/nifi-amqp-processors/src/test/java/org/apache/nifi/amqp/processors/ConsumeAMQPTest.java
Outdated
Show resolved
Hide resolved
...ndle/nifi-amqp-processors/src/test/java/org/apache/nifi/amqp/processors/ConsumeAMQPTest.java
Outdated
Show resolved
Hide resolved
...ndle/nifi-amqp-processors/src/test/java/org/apache/nifi/amqp/processors/ConsumeAMQPTest.java
Outdated
Show resolved
Hide resolved
...p-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/ConsumeAMQP.java
Outdated
Show resolved
Hide resolved
...p-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/ConsumeAMQP.java
Outdated
Show resolved
Hide resolved
...p-bundle/nifi-amqp-processors/src/main/java/org/apache/nifi/amqp/processors/ConsumeAMQP.java
Outdated
Show resolved
Hide resolved
|
Thanks, @exceptionfactory. The suggestions look good, and I have applied them to the PR. |
exceptionfactory
left a comment
There was a problem hiding this comment.
Thanks for the updates @umarhussain15, it looks like the latest commit missed updating a reference to HEADER_PREFIX, resulting in compilation failures. Can you correct that as well?
Co-authored-by: exceptionfactory <exceptionfactory@apache.org>
f696ece to
29744be
Compare
Codecov Report
@@ Coverage Diff @@
## main #7652 +/- ##
=======================================
Coverage ? 50.60%
Complexity ? 35103
=======================================
Files ? 5251
Lines ? 275605
Branches ? 30435
=======================================
Hits ? 139476
Misses ? 126049
Partials ? 10080 📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today! |
exceptionfactory
left a comment
There was a problem hiding this comment.
Thanks for working through the feedback @umarhussain15, the latest version looks good! +1 merging
Summary
NIFI-11995 extends ConsumeAMQP processor and allows the user to configure how to output headers from a received message. It provides three options:
amqp$headersattribute as string joined by the provided separator (Default) -> backward compatibleamqp$headersattribute as a valid JSON string. Headers with null value will be present as well in json with null value<Header Key Prefix>.<header key>attribute. It will not put headers which are null in message<Header Key Prefix>specifies the prefix value, defaults toconsume.amqpTest cases updated for new changes to the processor
Tracking
Please complete the following tracking steps prior to pull request creation.
Issue Tracking
Pull Request Tracking
NIFI-00000NIFI-00000Pull Request Formatting
mainbranchVerification
Please indicate the verification steps performed prior to pull request creation.
Build
mvn clean install -P contrib-checkLicensing
LICENSEandNOTICEfilesDocumentation