Skip to content

MINIFICPP-2700 Log errors if ContentRepository creation failed #2083

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
martinzink wants to merge 3 commits into
base: main
Choose a base branch
from
+26 −25
Open

MINIFICPP-2700 Log errors if ContentRepository creation failed #2083

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
aae5a65
MINIFICPP-2700 Log errors if ContentRepository creation failed
martinzink Jan 7, 2026
e09d59e
review changes
martinzink Jan 8, 2026
68da2bf
remove fail_safe option
martinzink Jan 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion libminifi/include/core/RepositoryFactory.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,11 @@ namespace org::apache::nifi::minifi::core {
* @param configuration_class_name configuration class name
* @param fail_safe determines whether or not to make the default class if configuration_class_name is invalid
Copy link
Contributor

@fgerlits fgerlits Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the documentation of the fail_safe parameter should be removed (also below, before the createRepository function)

* @param repo_name name of the repository
* @param logger pointer to a logger so it can log out errors
*/
std::unique_ptr<core::ContentRepository> createContentRepository(const std::string& configuration_class_name, bool fail_safe = false, const std::string& repo_name = "");
std::unique_ptr<core::ContentRepository> createContentRepository(const std::string& configuration_class_name,
const std::string& repo_name = "",
logging::Logger* logger = nullptr);

/**
* Create a repository represented by the configuration class name
Expand Down
44 changes: 21 additions & 23 deletions libminifi/src/core/RepositoryFactory.cpp
View file
Open in desktop
Copy link
Member

@szaszm szaszm Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be better to inject a logger as a function parameter instead of creating one locally for a semi-related class.

Copy link
Member Author

@martinzink martinzink Jan 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good idea, review changes

Original file line number Diff line number Diff line change
Expand Up @@ -28,33 +28,31 @@ using namespace std::literals::chrono_literals;

namespace org::apache::nifi::minifi::core {

std::unique_ptr<core::ContentRepository> createContentRepository(const std::string& configuration_class_name, bool fail_safe, const std::string& repo_name) {
std::unique_ptr<core::ContentRepository> createContentRepository(const std::string& configuration_class_name,
const std::string& repo_name,
logging::Logger* logger) {
std::string class_name_lc = configuration_class_name;
std::transform(class_name_lc.begin(), class_name_lc.end(), class_name_lc.begin(), ::tolower);
try {
auto return_obj = core::ClassLoader::getDefaultClassLoader().instantiate<core::ContentRepository>(class_name_lc,
class_name_lc);
if (return_obj) {
return_obj->setName(repo_name);
return return_obj;
}
if (class_name_lc == "volatilecontentrepository") {
return std::make_unique<core::repository::VolatileContentRepository>(repo_name);
} else if (class_name_lc == "filesystemrepository") {
return std::make_unique<core::repository::FileSystemRepository>(repo_name);
}
if (fail_safe) {
return std::make_unique<core::repository::VolatileContentRepository>("fail_safe");
} else {
throw std::runtime_error("Support for the provided configuration class could not be found");
}
} catch (const std::runtime_error&) {
if (fail_safe) {
return std::make_unique<core::repository::VolatileContentRepository>("fail_safe");
}

auto return_obj = core::ClassLoader::getDefaultClassLoader().instantiate<core::ContentRepository>(class_name_lc,
class_name_lc);
Copy link
Contributor

@fgerlits fgerlits Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this indentation looks bad; it may be better to keep this on one line

if (return_obj) {
return_obj->setName(repo_name);
return return_obj;
}

throw std::runtime_error("Support for the provided configuration class could not be found");
if (class_name_lc == "volatilecontentrepository") {
return std::make_unique<core::repository::VolatileContentRepository>(repo_name);
}
if (class_name_lc == "filesystemrepository") {
return std::make_unique<core::repository::FileSystemRepository>(repo_name);
}

logger->log_critical("Could not create the configured content repository ({})", configuration_class_name);
Copy link
Contributor

@fgerlits fgerlits Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we allow logger to be null, then please check it before dereferencing. It may be better to force it to be non-null?

if (class_name_lc == "databasecontentrepository") {
logger->log_error("To use DatabaseContentRepository MiNiFi needs RocksDB extension, please check the extension path configured in minifi.properties");
}
throw std::runtime_error("Support for the provided configuration class could not be found, check logs for more details");
}

std::unique_ptr<core::Repository> createRepository(const std::string& configuration_class_name, const std::string& repo_name) {
Expand Down
2 changes: 1 addition & 1 deletion minifi_main/MiNiFiMain.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ int main(int argc, char **argv) {

configure->get(minifi::Configure::nifi_content_repository_class_name, content_repo_class);

std::shared_ptr<core::ContentRepository> content_repo = core::createContentRepository(content_repo_class, true, "content");
std::shared_ptr<core::ContentRepository> content_repo = core::createContentRepository(content_repo_class, "content", logger.get());

if (!content_repo->initialize(configure)) {
logger->log_error("Content repository failed to initialize, exiting..");
Expand Down
Loading