You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2021/09/28 09:39:36 UTC
[GitHub] [nifi-minifi-cpp] martinzink opened a new pull request #1181: MINIFICPP-1650: ProcessSession::append sets the flowfile size
martinzink opened a new pull request #1181:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1181
incorrectly
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 main)?
- [ ] Is your initial contribution a single, squashed commit?
### For code changes:
- [ ] 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)?
- [ ] If applicable, have you updated the LICENSE file?
- [ ] If applicable, have you updated the NOTICE file?
### For documentation related changes:
- [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
### Note:
Please ensure that once the PR is submitted, you check GitHub Actions CI results for build issues and submit an update to your PR as soon as possible.
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #1181: MINIFICPP-1650: ProcessSession::append sets the flowfile size
Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on a change in pull request #1181:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1181#discussion_r719173733
##########
File path: libminifi/src/core/ProcessSession.cpp
##########
@@ -269,14 +269,15 @@ void ProcessSession::append(const std::shared_ptr<core::FlowFile> &flow, OutputS
}
// Call the callback to write the content
- size_t oldPos = stream->size();
+ size_t flow_file_size = flow->getSize();
+ size_t stream_size_before_callback = stream->size();
// this prevents an issue if we write, above, with zero length.
- if (oldPos > 0)
- stream->seek(oldPos + 1);
+ if (stream_size_before_callback > 0)
+ stream->seek(stream_size_before_callback + 1);
if (callback->process(stream) < 0) {
throw Exception(FILE_OPERATION_EXCEPTION, "Failed to process flowfile content");
}
- flow->setSize(stream->size());
+ flow->setSize(flow_file_size - stream_size_before_callback + stream->size());
Review comment:
nitpick: `flow_file_size + (stream->size() - stream_size_before_callback)` might convey more "intention"
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] martinzink commented on a change in pull request #1181: MINIFICPP-1650: ProcessSession::append sets the flowfile size
Posted by GitBox <gi...@apache.org>.
martinzink commented on a change in pull request #1181:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1181#discussion_r719196114
##########
File path: libminifi/src/core/ProcessSession.cpp
##########
@@ -269,14 +269,15 @@ void ProcessSession::append(const std::shared_ptr<core::FlowFile> &flow, OutputS
}
// Call the callback to write the content
- size_t oldPos = stream->size();
+ size_t flow_file_size = flow->getSize();
+ size_t stream_size_before_callback = stream->size();
// this prevents an issue if we write, above, with zero length.
- if (oldPos > 0)
- stream->seek(oldPos + 1);
+ if (stream_size_before_callback > 0)
+ stream->seek(stream_size_before_callback + 1);
if (callback->process(stream) < 0) {
throw Exception(FILE_OPERATION_EXCEPTION, "Failed to process flowfile content");
}
- flow->setSize(stream->size());
+ flow->setSize(flow_file_size - stream_size_before_callback + stream->size());
Review comment:
good idea, changed it in https://github.com/apache/nifi-minifi-cpp/pull/1181/commits/4debe9813863f259af20e6919a8521b5ef97d28c
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] martinzink commented on a change in pull request #1181: MINIFICPP-1650: ProcessSession::append sets the flowfile size
Posted by GitBox <gi...@apache.org>.
martinzink commented on a change in pull request #1181:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1181#discussion_r719129867
##########
File path: libminifi/src/core/ProcessSession.cpp
##########
@@ -269,14 +269,15 @@ void ProcessSession::append(const std::shared_ptr<core::FlowFile> &flow, OutputS
}
// Call the callback to write the content
- size_t oldPos = stream->size();
+ size_t flow_file_size = flow->getSize();
+ size_t stream_size_before_callback = stream->size();
// this prevents an issue if we write, above, with zero length.
- if (oldPos > 0)
- stream->seek(oldPos + 1);
+ if (stream_size_before_callback > 0)
+ stream->seek(stream_size_before_callback + 1);
if (callback->process(stream) < 0) {
throw Exception(FILE_OPERATION_EXCEPTION, "Failed to process flowfile content");
}
- flow->setSize(stream->size());
+ flow->setSize(flow_file_size - stream_size_before_callback + stream->size());
Review comment:
The problem was that ContentSession::write with WriteMode::APPEND can give back a new stream which it will merge with the original stream later. (see ContentSession::extendedResources_) However the flowfile size is only changed here. So appending to the flowfile overwrote the size with the appended content's size.
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] adamdebreceni closed pull request #1181: MINIFICPP-1650: ProcessSession::append sets the flowfile size
Posted by GitBox <gi...@apache.org>.
adamdebreceni closed pull request #1181:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1181
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi-minifi-cpp] martinzink commented on a change in pull request #1181: MINIFICPP-1650: ProcessSession::append sets the flowfile size
Posted by GitBox <gi...@apache.org>.
martinzink commented on a change in pull request #1181:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1181#discussion_r719129867
##########
File path: libminifi/src/core/ProcessSession.cpp
##########
@@ -269,14 +269,15 @@ void ProcessSession::append(const std::shared_ptr<core::FlowFile> &flow, OutputS
}
// Call the callback to write the content
- size_t oldPos = stream->size();
+ size_t flow_file_size = flow->getSize();
+ size_t stream_size_before_callback = stream->size();
// this prevents an issue if we write, above, with zero length.
- if (oldPos > 0)
- stream->seek(oldPos + 1);
+ if (stream_size_before_callback > 0)
+ stream->seek(stream_size_before_callback + 1);
if (callback->process(stream) < 0) {
throw Exception(FILE_OPERATION_EXCEPTION, "Failed to process flowfile content");
}
- flow->setSize(stream->size());
+ flow->setSize(flow_file_size - stream_size_before_callback + stream->size());
Review comment:
The problem was that ContentSession::write with WriteMode::APPEND can give back a new stream which it will merge with the original stream later. However the flowfile size is only changed here.
--
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org