lordgamez commented on code in PR #1490:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1490#discussion_r1094502554


##########
libminifi/include/core/Repository.h:
##########
@@ -54,28 +54,22 @@ constexpr auto MAX_REPOSITORY_STORAGE_SIZE = 10_MiB;
 constexpr auto MAX_REPOSITORY_ENTRY_LIFE_TIME = std::chrono::minutes(10);
 constexpr auto REPOSITORY_PURGE_PERIOD = std::chrono::milliseconds(2500);
 
-class Repository : public core::CoreComponent {
+class Repository : public core::ReportableRepository {

Review Comment:
   You are right, I also felt this should be the other way around, but the root 
cause of this problem is the current namings of the repositories. The 
`core::Repository` class should be the most generic class for all repositories, 
but it is only the base for the flowfile and provenance repositories. 
`ContentRepository` was entirely separate from the hierarchy starting with 
`core::Repository` so I had to come up with something new as I didn't want to 
change the current class names (core::Repository is used way too many times and 
I didn't want to blow up this PR). 
   
   Maybe we can rename the `ReportableRepository` to `Repository` and come up 
with new names to the classes we already have. The main difference seems to be 
that `core::Repository` and its children are discrete key based storage 
repositories with put, multiput, delete, get operations while 
`ContentRepository` is stream based repositories with read, write operations. 
So the `Repository` could be renamed to `KeyValueRepository` which is inherited 
by `ThreadedKeyValueRepository`, but I would create a separate PR for that.
   
   Do you think this is viable?



-- 
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

Reply via email to