You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by pipern <gi...@git.apache.org> on 2017/05/14 12:24:24 UTC
[GitHub] nifi pull request #1795: NIFI-1490 Add multipart request support to ListenHT...
GitHub user pipern opened a pull request:
https://github.com/apache/nifi/pull/1795
NIFI-1490 Add multipart request support to ListenHTTP Processor
### For all changes:
- [X] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
- [X] 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.
- [X] Has your PR been rebased against the latest commit within the target branch (typically master)?
- [X] Is your initial contribution a single, squashed commit?
### For code changes:
- [ ] Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi folder?
- [X] Have you written or updated unit tests to verify your changes?
- [X] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [X] If applicable, have you updated the LICENSE file, including the main LICENSE file under nifi-assembly?
- [X] If applicable, have you updated the NOTICE file, including the main NOTICE file found under nifi-assembly?
- [ ] If adding new Properties, have you added .displayName in addition to .name (programmatic access) for each of the new properties?
### For documentation related changes:
- [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/pipern/nifi master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/nifi/pull/1795.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1795
----
commit e7d4e3c5ed109c3287122c4fa4b2268e8fa4eed7
Author: Nick Piper <ni...@cgi.com>
Date: 2017-05-14T12:17:36Z
Assert that messages were transfered to ListenHTTP's relationship rather than ListenTCP (seems more semantically correct)
commit b0aace531ef1753bd296634da2f36932494d144e
Author: Nick Piper <ni...@cgi.com>
Date: 2017-05-14T12:18:31Z
NIFI-1490 Add multipart request support to ListenHTTP Processor
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #1795: NIFI-1490 Add multipart request support to ListenHTTP Proc...
Posted by pipern <gi...@git.apache.org>.
Github user pipern commented on the issue:
https://github.com/apache/nifi/pull/1795
Do you think it's worth it @joewitt , since maybe this PR should be rejected and someone works on ExtractMultiPartHttp instead? (I'd like to, but can't commit to it. Maybe over Christmas...)
The decision of ExtractMultiPartHttp vs. an extracting-ListenHTTP maybe comes down to how we want errors send back to the client. If we want to be able to tell the HTTP client that their MultiPart was not well-formed, we'd have to do it in ListenHTTP, right? (Which potentially invalidates my first paragraph.)
---
[GitHub] nifi issue #1795: NIFI-1490 Add multipart request support to ListenHTTP Proc...
Posted by joewitt <gi...@git.apache.org>.
Github user joewitt commented on the issue:
https://github.com/apache/nifi/pull/1795
@pipern Do you mind rebasing and pushing the latest ?
Apologies for this falling through the review cracks. If anyone has time soon please do take a look/test.
---
[GitHub] nifi issue #1795: NIFI-1490 Add multipart request support to ListenHTTP Proc...
Posted by pipern <gi...@git.apache.org>.
Github user pipern commented on the issue:
https://github.com/apache/nifi/pull/1795
Thanks for taking a look @joewitt .
The old behaviour is persisted if _Multipart part name pattern_ property is not set (and not set is the default.)
The current implementation in this PR does only grab the first matching Part. It's an obvious next step to grab all matching parts, and indeed, maybe it's better to wait and improve this before merging, so that the behaviour doesn't change later. The implementation in the PR meets our own need, so unsure if we'll get chance to revisit and make the improvement soon.
I can also imagine an implementation which puts some parts into content, and some parts into attributes. Maybe a step towards that is, building on the current PR, to:
* Rename _Multipart part name pattern_ as _Multipart file part name pattern_ (and then generate several flowfiles, one for each matching part)
* Add _Multipart attribute part name pattern_ (and then, for each otherwise generated flowfile, put these parts as attributes)
> ExtractMultiPartHttp for example could take as input the entire request body in a flowfile that came from ListenHTTP and this new processor could create child objects which are each of the parts.
This would also need the ContentType header to be kept by the ListenHTTP (which it can do.)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #1795: NIFI-1490 Add multipart request support to ListenHTTP Proc...
Posted by pipern <gi...@git.apache.org>.
Github user pipern commented on the issue:
https://github.com/apache/nifi/pull/1795
Unfortunately appveyor test is failing - https://ci.appveyor.com/project/ApacheSoftwareFoundation/nifi/build/1.0.0-SNAPSHOT-5108
I don't think it appears related:
```
Running org.wali.TestMinimalLockingWriteAheadLog
Tests run: 12, Failures: 0, Errors: 1, Skipped: 2, Time elapsed: 2.828 sec <<< FAILURE! - in org.wali.TestMinimalLockingWriteAheadLog
testRecoverFileThatHasTrailingNULBytesAndTruncation(org.wali.TestMinimalLockingWriteAheadLog) Time elapsed: 0 sec <<< ERROR!
java.nio.channels.OverlappingFileLockException: null
at sun.nio.ch.SharedFileLockTable.checkList(FileLockTable.java:255)
at sun.nio.ch.SharedFileLockTable.add(FileLockTable.java:152)
at sun.nio.ch.FileChannelImpl.lock(FileChannelImpl.java:1063)
at java.nio.channels.FileChannel.lock(FileChannel.java:1053)
at org.wali.MinimalLockingWriteAheadLog.<init>(MinimalLockingWriteAheadLog.java:187)
at org.wali.MinimalLockingWriteAheadLog.<init>(MinimalLockingWriteAheadLog.java:108)
at org.wali.TestMinimalLockingWriteAheadLog.testRecoverFileThatHasTrailingNULBytesAndTruncation(TestMinimalLockingWriteAheadLog.java:472)
```
but I also don't think it got as far as running the ListenHTTP test.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] nifi issue #1795: NIFI-1490 Add multipart request support to ListenHTTP Proc...
Posted by joewitt <gi...@git.apache.org>.
Github user joewitt commented on the issue:
https://github.com/apache/nifi/pull/1795
hello @pipern. Thanks for contributing. If i'm reading this right it looks like what this will do is grab the first matching section of a multi-part request. The current behavior before the patch would be to, i believe, grab all of the request as the flow file content. Given that, it seems like rather than altering this processor to grab just the first match we'd either want to alter it to grab each section as new flow files OR perhaps better yet have a different processor which understands how to extract out the specific parts into individual flowfiles. ExtractMultiPartHttp for example could take as input the entire request body in a flowfile that came from ListenHTTP and this new processor could create child objects which are each of the parts.
What do you think about that?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---