MINIFICPP-1244 Support the Initial Start Position property in TailFile#1052
MINIFICPP-1244 Support the Initial Start Position property in TailFile#1052lordgamez wants to merge 22 commits intoapache:mainfrom
Conversation
| processAllRotatedFiles(session, state); | ||
| } else if (initial_start_position_ == "Current Time") { | ||
| state.position_ = utils::file::FileUtils::file_size(full_file_name); | ||
| state.last_read_time_ = std::chrono::system_clock::now(); |
There was a problem hiding this comment.
I think we also need to compute the checksum of the file up to this point. Otherwise, when
- MiNiFi starts with Current Time at the middle of
test.log onTrigger()reads a few more lines (and computes the checksum, but only from the middle)test.loggets rolled over totest.log.1onTrigger()gets called again, it will compute the checksum oftest.log.1from the beginning, and the checksum will not match the state, so it will re-readtest.log.1from the beginning rather than from the point where it left off.
There was a problem hiding this comment.
Good point, I added this use case as a separate test and added the fix in 1eaf3d3
There was a problem hiding this comment.
I would use utils::file::FileUtils::computeChecksum() as it is done in findRotatedFiles() (line 656), mainly for consistency, but it's also slightly shorter/simpler.
There was a problem hiding this comment.
Thanks for the tip, I totally missed this, updated in 7a926a3
There was a problem hiding this comment.
For some reason on Windows the test case fails, have to investigate it further
There was a problem hiding this comment.
It seems that as I previously created a rolled over file for all tests which was later rewritten in the rollover scenario with the same filename and it was identified as a new file on Windows. Extracted the new test as a separate test case in cbd9e21 where I removed this from the test setup.
lordgamez
force-pushed
the
MINIFICPP-1244
branch
from
11ee75a to
221b1d7
Compare
| } | ||
| return utils::nullopt; | ||
| } | ||
| std::string getRequiredPropertyOrThrow(const core::ProcessContext* context, const std::string& property_name); |
There was a problem hiding this comment.
I was under the impression that getProperty already throws on missing required properties
here
There was a problem hiding this comment.
It looks like it, maybe @hunyadi-dev can comment on it as it was introduced in #940. Are there any differences in the behavior of getProperty and getRequiredPropertyOrThrow?
lordgamez
force-pushed
the
MINIFICPP-1244
branch
from
393f540 to
f6a130f
Compare
Closes apache#1052 Signed-off-by: Marton Szasz <szaszm01@gmail.com>
4 participants
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:
For documentation related changes:
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.