MINIFICPP-748 - Nanofi: add custom C processor example #499
Conversation
|
|
||
| char md5value[50]; | ||
|
|
||
| snprintf(md5value, attr.value_size, "%s", (const char*)attr.value+1); |
There was a problem hiding this comment.
As a user I find it pretty painful to use.
@phrocker I think we should consider changing values to "const char *"
Not sure what's the right timing for that release-wise.
If I remember well we already have a Jira for that.
There was a problem hiding this comment.
Totally agree -- and no objection from my side since it's an internal struct and assignment likely wouldn't cause any impact to currently existing code.
You're right though, I do think there may be a JIRA for it.
There was a problem hiding this comment.
Just found this: https://issues.apache.org/jira/browse/MINIFICPP-646
This might fit.
However my question was mostly about timing: this is most probably going to be a breaking change in the API and 0.6.0 is to be released soon, I wonder if we try to add it or do it later in 0.7.0 or 1.0? The later we do it the more we break.
| *content_repo); | ||
| auto stream = (*content_repo)->read(claim); | ||
| return stream->read(target, size); | ||
| } else { |
There was a problem hiding this comment.
The problem here is that ff->contentlocation is used for two different purpose:
-A simple filename to show where the content of the flowfile in the filesystem is
-An internal string that represent the location of the file within the content repo
I think this should be separated.
@phrocker do you have any longer term plan to resolve this in nanofi?
There was a problem hiding this comment.
Imagine if there were no c++ involved, and as a result, no content repo, would that change your concern?
Is this not an internal struct? it's fair to expect interaction with our structures to not be raw access. As a result it's also fair to add a temporary item while content-repo is in existence. I'll leave that up to you, but maybe we should make clear that the FF object is not intended to be access outside of the bounds of function accessors?
There was a problem hiding this comment.
I wasn't aware of the Nanofi related visions here.
In case we don't plan to support content repo in Nanofi, just have flow files store content in a simple file, that's fine. Although I would expect people in IoT world to use Nanofi to trasmit data without any disk i/o operations.
As this struct is most probably going to change anyway in the near future, it's fine as is.
This closes apache#499. Signed-off-by: Marc Parisi <phrocker@apache.org>
Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.
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 MINIFICPP-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 master)?
Is your initial contribution a single, squashed commit?
For code changes:
For documentation related changes:
Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.