szaszm commented on code in PR #1670:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1670#discussion_r1338571913
##########
extensions/aws/s3/S3Wrapper.cpp:
##########
@@ -324,7 +324,7 @@ std::optional<std::vector<ListedObjectAttributes>>
S3Wrapper::listObjects(const
return std::nullopt;
}
const auto& objects = aws_result->GetContents();
- logger_->log_debug("AWS S3 List operation returned %zu objects. This
result is%s truncated.", objects.size(), aws_result->GetIsTruncated() ? "" :
"not");
+ logger_->log_debug("AWS S3 List operation returned {} objects. This result
is{} truncated.", objects.size(), aws_result->GetIsTruncated() ? "" : "not");
Review Comment:
```suggestion
logger_->log_debug("AWS S3 List operation returned {} objects. This
result is{} truncated.", objects.size(), aws_result->GetIsTruncated() ? "" : "
not");
```
##########
extensions/aws/utils/AWSSdkLogger.cpp:
##########
@@ -21,51 +21,45 @@
#include "aws/core/utils/logging/LogLevel.h"
-namespace org {
-namespace apache {
-namespace nifi {
-namespace minifi {
-namespace aws {
-namespace utils {
+namespace org::apache::nifi::minifi::aws::utils {
-Aws::Utils::Logging::LogLevel AWSSdkLogger::GetLogLevel() const {
- if (logger_->should_log(minifi::core::logging::LOG_LEVEL::trace))
- return Aws::Utils::Logging::LogLevel::Trace;
- if (logger_->should_log(minifi::core::logging::LOG_LEVEL::debug))
- return Aws::Utils::Logging::LogLevel::Debug;
- if (logger_->should_log(minifi::core::logging::LOG_LEVEL::info))
- return Aws::Utils::Logging::LogLevel::Info;
- if (logger_->should_log(minifi::core::logging::LOG_LEVEL::warn))
- return Aws::Utils::Logging::LogLevel::Warn;
- if (logger_->should_log(minifi::core::logging::LOG_LEVEL::err))
- return Aws::Utils::Logging::LogLevel::Error;
- if (logger_->should_log(minifi::core::logging::LOG_LEVEL::critical))
- return Aws::Utils::Logging::LogLevel::Fatal;
- return Aws::Utils::Logging::LogLevel::Off;
+namespace {
+Aws::Utils::Logging::LogLevel mapToAwsLevels(core::logging::LOG_LEVEL level) {
+ switch (level) {
+ case core::logging::trace: return Aws::Utils::Logging::LogLevel::Trace;
+ case core::logging::debug: return Aws::Utils::Logging::LogLevel::Debug;
+ case core::logging::info: return Aws::Utils::Logging::LogLevel::Info;
+ case core::logging::warn: return Aws::Utils::Logging::LogLevel::Warn;
+ case core::logging::err: return Aws::Utils::Logging::LogLevel::Error;
+ case core::logging::critical: return Aws::Utils::Logging::LogLevel::Fatal;
+ case core::logging::off: return Aws::Utils::Logging::LogLevel::Off;
Review Comment:
```suggestion
switch (level) {
using core::logging::LOG_LEVEL;
using AwsLogLevel = Aws::Utils::Logging::LogLevel;
case LOG_LEVEL::trace: return AwsLogLevel::Trace;
case LOG_LEVEL::debug: return AwsLogLevel::Debug;
case LOG_LEVEL::info: return AwsLogLevel::Info;
case LOG_LEVEL::warn: return AwsLogLevel::Warn;
case LOG_LEVEL::err: return AwsLogLevel::Error;
case LOG_LEVEL::critical: return AwsLogLevel::Fatal;
case LOG_LEVEL::off: return AwsLogLevel::Off;
```
##########
extensions/aws/utils/AWSSdkLogger.cpp:
##########
@@ -21,51 +21,45 @@
#include "aws/core/utils/logging/LogLevel.h"
-namespace org {
-namespace apache {
-namespace nifi {
-namespace minifi {
-namespace aws {
-namespace utils {
+namespace org::apache::nifi::minifi::aws::utils {
-Aws::Utils::Logging::LogLevel AWSSdkLogger::GetLogLevel() const {
- if (logger_->should_log(minifi::core::logging::LOG_LEVEL::trace))
- return Aws::Utils::Logging::LogLevel::Trace;
- if (logger_->should_log(minifi::core::logging::LOG_LEVEL::debug))
- return Aws::Utils::Logging::LogLevel::Debug;
- if (logger_->should_log(minifi::core::logging::LOG_LEVEL::info))
- return Aws::Utils::Logging::LogLevel::Info;
- if (logger_->should_log(minifi::core::logging::LOG_LEVEL::warn))
- return Aws::Utils::Logging::LogLevel::Warn;
- if (logger_->should_log(minifi::core::logging::LOG_LEVEL::err))
- return Aws::Utils::Logging::LogLevel::Error;
- if (logger_->should_log(minifi::core::logging::LOG_LEVEL::critical))
- return Aws::Utils::Logging::LogLevel::Fatal;
- return Aws::Utils::Logging::LogLevel::Off;
+namespace {
+Aws::Utils::Logging::LogLevel mapToAwsLevels(core::logging::LOG_LEVEL level) {
+ switch (level) {
+ case core::logging::trace: return Aws::Utils::Logging::LogLevel::Trace;
+ case core::logging::debug: return Aws::Utils::Logging::LogLevel::Debug;
+ case core::logging::info: return Aws::Utils::Logging::LogLevel::Info;
+ case core::logging::warn: return Aws::Utils::Logging::LogLevel::Warn;
+ case core::logging::err: return Aws::Utils::Logging::LogLevel::Error;
+ case core::logging::critical: return Aws::Utils::Logging::LogLevel::Fatal;
+ case core::logging::off: return Aws::Utils::Logging::LogLevel::Off;
+ default:
+ throw std::invalid_argument(fmt::format("Invalid LOG_LEVEL {}",
magic_enum::enum_underlying(level)));
+ }
}
-void AWSSdkLogger::Log(Aws::Utils::Logging::LogLevel log_level, const char*
tag, const char* format_str, ...) { // NOLINT(cert-dcl50-cpp)
- switch (log_level) {
- case Aws::Utils::Logging::LogLevel::Trace:
- logger_->log_trace("[%s] %s", tag, format_str);
- break;
- case Aws::Utils::Logging::LogLevel::Debug:
- logger_->log_debug("[%s] %s", tag, format_str);
- break;
- case Aws::Utils::Logging::LogLevel::Info:
- logger_->log_info("[%s] %s", tag, format_str);
- break;
- case Aws::Utils::Logging::LogLevel::Warn:
- logger_->log_warn("[%s] %s", tag, format_str);
- break;
- case Aws::Utils::Logging::LogLevel::Error:
- case Aws::Utils::Logging::LogLevel::Fatal:
- logger_->log_error("[%s] %s", tag, format_str);
- break;
+core::logging::LOG_LEVEL mapFromAwsLevels(Aws::Utils::Logging::LogLevel level)
{
+ switch (level) {
+ case Aws::Utils::Logging::LogLevel::Off: return core::logging::off;
+ case Aws::Utils::Logging::LogLevel::Fatal: return core::logging::critical;
+ case Aws::Utils::Logging::LogLevel::Error:return core::logging::err;
+ case Aws::Utils::Logging::LogLevel::Warn: return core::logging::warn;
+ case Aws::Utils::Logging::LogLevel::Info: return core::logging::info;
+ case Aws::Utils::Logging::LogLevel::Debug: return core::logging::debug;
+ case Aws::Utils::Logging::LogLevel::Trace: return core::logging::trace;
default:
- break;
+ throw std::invalid_argument(fmt::format("Invalid
Aws::Utils::Logging::LogLevel {}", magic_enum::enum_underlying(level)));
Review Comment:
I'd omit the default case, and move the `throw` outside the switch. That way
the compiler can warn if not all enum values are handled.
##########
extensions/http-curl/client/HTTPCallback.h:
##########
@@ -142,8 +142,8 @@ class HttpStreamingCallback final : public
utils::HTTPUploadByteArrayInputCallba
current_buffer_start_ = total_bytes_loaded_;
current_pos_ = current_buffer_start_;
total_bytes_loaded_ += current_vec_.size();
- logger_->log_trace("loadNextBuffer() loaded new buffer, ptr_: %p, size:
%zu, current_buffer_start_: %zu, current_pos_: %zu, total_bytes_loaded_: %zu",
- ptr_,
+ logger_->log_trace("loadNextBuffer() loaded new buffer, ptr_: {}, size:
{}, current_buffer_start_: {}, current_pos_: {}, total_bytes_loaded_: {}",
+ static_cast<void*>(ptr_),
Review Comment:
Why is the cast to `void*` needed?
##########
extensions/http-curl/processors/InvokeHTTP.cpp:
##########
@@ -100,7 +100,7 @@ void InvokeHTTP::setupMembersFromProperties(const
core::ProcessContext& context)
attributes_to_send_ = context.getProperty(AttributesToSend)
| utils::filter([](const std::string& s) { return
!s.empty(); }) // avoid compiling an empty string to regex
| utils::transform([](const std::string& regex_str) {
return utils::Regex{regex_str}; })
- | utils::orElse([this] { logger_->log_debug("%s is
missing, so the default value will be used",
std::string{AttributesToSend.name}); });
+ | utils::orElse([this] { logger_->log_debug("{} is
missing, so the default value will be used",
std::string{AttributesToSend.name}); });
Review Comment:
Wrapping in `std::string` is no longer necessary. There's a lot of these,
and I think it would be nice to fix them here.
```suggestion
| utils::orElse([this] { logger_->log_debug("{} is
missing, so the default value will be used", AttributesToSend.name); });
```
--
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
