fgerlits commented on code in PR #1631:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1631#discussion_r1329969207
##########
libminifi/test/unit/LoggerTests.cpp:
##########
@@ -276,7 +280,7 @@ TEST_CASE("Setting either properties to 0 disables
in-memory compressed logs", "
log_config.initialize(properties);
auto logger = log_config.getLogger("DisableCompressionTestLogger");
logger->log_error("Hi there");
- REQUIRE((logging::LoggerConfiguration::getCompressedLog(true) == nullptr) ==
is_nullptr);
+ REQUIRE(logging::LoggerConfiguration::getCompressedLogs(true).empty() ==
is_nullptr);
Review Comment:
it would be better to rename `is_nullptr` to `is_empty`
##########
libminifi/include/core/logging/internal/LogCompressorSink.h:
##########
@@ -57,26 +58,44 @@ class LogCompressorSink : public
spdlog::sinks::base_sink<std::mutex> {
~LogCompressorSink() override;
template<class Rep, class Period>
- std::unique_ptr<io::InputStream> getContent(const std::chrono::duration<Rep,
Period>& time, bool flush = false) {
+ std::vector<std::unique_ptr<io::InputStream>> getContent(const
std::chrono::duration<Rep, Period>& time, bool flush = false) {
if (flush) {
cached_logs_.commit();
compress(true);
}
- LogBuffer compressed;
- if (!compressed_logs_.tryDequeue(compressed, time) && flush) {
- return createEmptyArchive();
+
+ std::vector<std::unique_ptr<io::InputStream>> log_segments;
+ const auto segment_count = compressed_logs_.itemCount();
+ for (size_t i = 0; i < segment_count; ++i) {
+ LogBuffer compressed;
+ if (!compressed_logs_.tryDequeue(compressed, time) && flush) {
+ break;
Review Comment:
I don't understand the purpose of checking `flush` here (I didn't understand
it before this change either, but it got more complex). As far as I can see,
we only call this with `flush = true`.
Can we remove the `flush` argument? If not, then I think the logic needs to
be made clearer, either by a comment or some other way.
--
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-unsubscr...@nifi.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
