NIFI-9277: Add Record Reader and Writer to ListenHTTP#5446
NIFI-9277: Add Record Reader and Writer to ListenHTTP#5446Lehel44 wants to merge 4 commits intoapache:mainfrom
Conversation
...e/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ListenHTTP.java
Outdated
Show resolved
Hide resolved
exceptionfactory
left a comment
There was a problem hiding this comment.
Thanks for this new feature @Lehel44! It seems like the primary value of adding a RecordReader is to provide some amount of input request validation that influences the response sent to the client, but it wasn't clear if that was happening. I noted the catching and logging of exceptions in processRecord().
...processors/src/main/java/org/apache/nifi/processors/standard/servlets/ListenHTTPServlet.java
Outdated
Show resolved
Hide resolved
...fi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestListenHTTP.java
Show resolved
Hide resolved
… test case with Record Reader.
...processors/src/main/java/org/apache/nifi/processors/standard/servlets/ListenHTTPServlet.java
Show resolved
Hide resolved
...e/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ListenHTTP.java
Show resolved
Hide resolved
...processors/src/main/java/org/apache/nifi/processors/standard/servlets/ListenHTTPServlet.java
Outdated
Show resolved
Hide resolved
...processors/src/main/java/org/apache/nifi/processors/standard/servlets/ListenHTTPServlet.java
Outdated
Show resolved
Hide resolved
| writer.write(record); | ||
| } | ||
| } | ||
| } catch (IOException | MalformedRecordException | SchemaNotFoundException e) { |
There was a problem hiding this comment.
This exception handling seems too broad. MalformedRecordException is probably the only one that should warrant a 400.
There was a problem hiding this comment.
The IOException here may be thrown by ReaderFactory::createRecordReader or WriterFactory::createRecordWriter or RecordWriter::write. In case of JsonRecordReader this can be a JsonParseException (which is an IOException). I think in case of IOException we might return with 400 (BAD_REQUEST )return code. I separated the SchemaNotFoundException which returns 500 (INTERNAL_SERVER_ERROR).
…fined exception handling in ListenHttpServlet::processRecord.
|
LGTM |
This closes apache#5446. Signed-off-by: Tamas Palfy <tamas.bertalan.palfy@gmail.com>
Thank you for submitting a contribution to Apache NiFi.
Please provide a short description of the PR here:
Description of PR
Added record reader and writer to ListenHTTP processor:
JIRA:
https://issues.apache.org/jira/browse/NIFI-9277
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
For all changes:
Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
Has your PR been rebased against the latest commit within the target branch (typically
main)?Is your initial contribution a single, squashed commit? Additional commits in response to PR reviewer feedback should be made on this branch and pushed to allow change tracking. Do not
squashor use--forcewhen pushing to allow for clean monitoring of changes.For code changes:
mvn -Pcontrib-check clean installat the rootnififolder?LICENSEfile, including the mainLICENSEfile undernifi-assembly?NOTICEfile, including the mainNOTICEfile found undernifi-assembly?.displayNamein addition to .name (programmatic access) for each of the new properties?For documentation related changes:
Note:
Please ensure that once the PR is submitted, you check GitHub Actions CI for build issues and submit an update to your PR as soon as possible.