You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2020/07/31 12:39:50 UTC

[GitHub] [nifi-minifi-cpp] adamdebreceni opened a new pull request #857: MINIFICPP-1309 - RAII based resourceClaim management

adamdebreceni opened a new pull request #857:
URL: https://github.com/apache/nifi-minifi-cpp/pull/857


   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 master)?
   
   - [ ] Is your initial contribution a single, squashed commit?
   
   ### For code changes:
   - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
   - [ ] If applicable, have you updated the LICENSE file?
   - [ ] If applicable, have you updated the NOTICE file?
   
   ### For documentation related changes:
   - [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
   
   ### Note:
   Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.
   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #857: MINIFICPP-1309 - RAII based resourceClaim management

Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #857:
URL: https://github.com/apache/nifi-minifi-cpp/pull/857#discussion_r481046465



##########
File path: libminifi/src/ResourceClaim.cpp
##########
@@ -41,22 +41,28 @@ void setDefaultDirectory(std::string path) {
 }
 
 ResourceClaim::ResourceClaim(std::shared_ptr<core::StreamManager<ResourceClaim>> claim_manager)
-    : claim_manager_(claim_manager),
-      deleted_(false),
-      logger_(logging::LoggerFactory<ResourceClaim>::getLogger()) {
-  auto contentDirectory = claim_manager_->getStoragePath();
-  if (contentDirectory.empty())
-    contentDirectory = default_directory_path;
+    : claim_manager_(std::move(claim_manager)),
+      logger_(logging::LoggerFactory<ResourceClaim>::getLogger()),
+      _contentFullPath(([&] {
+        auto contentDirectory = claim_manager->getStoragePath();

Review comment:
       This is a null pointer dereference after `claim_manager` is moved-from in line 44. Use the member `claim_manager_` to avoid this.
   
   The lambda is unnecessarily wrapped in parentheses. You can call a lambda without wrapping it in parens, like this: `[](){}()`




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi-minifi-cpp] arpadboda closed pull request #857: MINIFICPP-1309 - RAII based resourceClaim management

Posted by GitBox <gi...@apache.org>.
arpadboda closed pull request #857:
URL: https://github.com/apache/nifi-minifi-cpp/pull/857


   


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi-minifi-cpp] fgerlits commented on pull request #857: MINIFICPP-1309 - RAII based resourceClaim management

Posted by GitBox <gi...@apache.org>.
fgerlits commented on pull request #857:
URL: https://github.com/apache/nifi-minifi-cpp/pull/857#issuecomment-669814958


   Is the Windows (GitHub actions) test failure related to this change?  It doesn't look like one of the usual flaky tests.


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #857: MINIFICPP-1309 - RAII based resourceClaim management

Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on a change in pull request #857:
URL: https://github.com/apache/nifi-minifi-cpp/pull/857#discussion_r481060518



##########
File path: libminifi/src/ResourceClaim.cpp
##########
@@ -41,22 +41,28 @@ void setDefaultDirectory(std::string path) {
 }
 
 ResourceClaim::ResourceClaim(std::shared_ptr<core::StreamManager<ResourceClaim>> claim_manager)
-    : claim_manager_(claim_manager),
-      deleted_(false),
-      logger_(logging::LoggerFactory<ResourceClaim>::getLogger()) {
-  auto contentDirectory = claim_manager_->getStoragePath();
-  if (contentDirectory.empty())
-    contentDirectory = default_directory_path;
+    : claim_manager_(std::move(claim_manager)),
+      logger_(logging::LoggerFactory<ResourceClaim>::getLogger()),
+      _contentFullPath(([&] {
+        auto contentDirectory = claim_manager->getStoragePath();

Review comment:
       actually the initializers are executed in the order of their declaration, and here they are out of sync with the initializer list's order, will change the order to represent the declaration order




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #857: MINIFICPP-1309 - RAII based resourceClaim management

Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #857:
URL: https://github.com/apache/nifi-minifi-cpp/pull/857#discussion_r481038617



##########
File path: libminifi/include/ResourceClaim.h
##########
@@ -45,8 +45,10 @@ extern std::string default_directory_path;
 extern void setDefaultDirectory(std::string);
 
 // ResourceClaim Class
-class ResourceClaim : public std::enable_shared_from_this<ResourceClaim> {
+class ResourceClaim {
  public:
+  // the type which uniquely represents the resource for the owning manager
+  using Path = std::string;

Review comment:
       No, it's fine this way. Thanks for fixing the ctor.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #857: MINIFICPP-1309 - RAII based resourceClaim management

Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on a change in pull request #857:
URL: https://github.com/apache/nifi-minifi-cpp/pull/857#discussion_r466275440



##########
File path: libminifi/include/ResourceClaim.h
##########
@@ -47,6 +47,8 @@ extern void setDefaultDirectory(std::string);
 // ResourceClaim Class
 class ResourceClaim : public std::enable_shared_from_this<ResourceClaim> {

Review comment:
       removed




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi-minifi-cpp] fgerlits commented on a change in pull request #857: MINIFICPP-1309 - RAII based resourceClaim management

Posted by GitBox <gi...@apache.org>.
fgerlits commented on a change in pull request #857:
URL: https://github.com/apache/nifi-minifi-cpp/pull/857#discussion_r464910752



##########
File path: libminifi/test/persistence-tests/PersistenceTests.cpp
##########
@@ -257,8 +258,26 @@ TEST_CASE("Persisted flowFiles are updated on modification", "[TestP1]") {
   config->set(minifi::Configure::nifi_flowfile_repository_directory_default, utils::file::FileUtils::concat_path(dir, "flowfile_repository"));
 
   std::shared_ptr<core::Repository> prov_repo = std::make_shared<TestRepository>();
-  std::shared_ptr<core::repository::FlowFileRepository> ff_repository = std::make_shared<core::repository::FlowFileRepository>("flowFileRepository");
-  std::shared_ptr<core::ContentRepository> content_repo = std::make_shared<core::repository::FileSystemRepository>();
+  std::shared_ptr<core::Repository> ff_repository;
+  std::shared_ptr<core::ContentRepository> content_repo;
+  SECTION("FlowFileRepository"){
+    ff_repository = std::make_shared<core::repository::FlowFileRepository>("flowFileRepository");
+    SECTION("VolatileContentRepository"){
+      content_repo = std::make_shared<core::repository::VolatileContentRepository>();
+    }
+    SECTION("FileSystemContenRepository"){
+      content_repo = std::make_shared<core::repository::FileSystemRepository>();
+    }
+  }
+  SECTION("VolatileFlowFileRepository"){
+    ff_repository = std::make_shared<core::repository::FlowFileRepository>("volatileFlowFileRepository");

Review comment:
       should this be an instance of `VolatileFlowFileRepository`?

##########
File path: libminifi/include/ResourceClaim.h
##########
@@ -47,6 +47,8 @@ extern void setDefaultDirectory(std::string);
 // ResourceClaim Class
 class ResourceClaim : public std::enable_shared_from_this<ResourceClaim> {

Review comment:
       is `enable_shared_from_this` still needed?

##########
File path: libminifi/test/persistence-tests/PersistenceTests.cpp
##########
@@ -257,8 +258,26 @@ TEST_CASE("Persisted flowFiles are updated on modification", "[TestP1]") {
   config->set(minifi::Configure::nifi_flowfile_repository_directory_default, utils::file::FileUtils::concat_path(dir, "flowfile_repository"));
 
   std::shared_ptr<core::Repository> prov_repo = std::make_shared<TestRepository>();
-  std::shared_ptr<core::repository::FlowFileRepository> ff_repository = std::make_shared<core::repository::FlowFileRepository>("flowFileRepository");
-  std::shared_ptr<core::ContentRepository> content_repo = std::make_shared<core::repository::FileSystemRepository>();
+  std::shared_ptr<core::Repository> ff_repository;
+  std::shared_ptr<core::ContentRepository> content_repo;
+  SECTION("FlowFileRepository"){
+    ff_repository = std::make_shared<core::repository::FlowFileRepository>("flowFileRepository");
+    SECTION("VolatileContentRepository"){
+      content_repo = std::make_shared<core::repository::VolatileContentRepository>();
+    }
+    SECTION("FileSystemContenRepository"){

Review comment:
       typo: `Conten` -> `Content` (or change to `FileSystemRepository`?) -- also in the other section




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #857: MINIFICPP-1309 - RAII based resourceClaim management

Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #857:
URL: https://github.com/apache/nifi-minifi-cpp/pull/857#discussion_r481046465



##########
File path: libminifi/src/ResourceClaim.cpp
##########
@@ -41,22 +41,28 @@ void setDefaultDirectory(std::string path) {
 }
 
 ResourceClaim::ResourceClaim(std::shared_ptr<core::StreamManager<ResourceClaim>> claim_manager)
-    : claim_manager_(claim_manager),
-      deleted_(false),
-      logger_(logging::LoggerFactory<ResourceClaim>::getLogger()) {
-  auto contentDirectory = claim_manager_->getStoragePath();
-  if (contentDirectory.empty())
-    contentDirectory = default_directory_path;
+    : claim_manager_(std::move(claim_manager)),
+      logger_(logging::LoggerFactory<ResourceClaim>::getLogger()),
+      _contentFullPath(([&] {
+        auto contentDirectory = claim_manager->getStoragePath();

Review comment:
       This is a null pointer dereference after `claim_manager` is moved-from in line 44. Use the member `claim_manager_` to avoid this.
   
   The lambda is unnecessarily wrapped in parentheses.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #857: MINIFICPP-1309 - RAII based resourceClaim management

Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #857:
URL: https://github.com/apache/nifi-minifi-cpp/pull/857#discussion_r481059743



##########
File path: libminifi/include/ResourceClaim.h
##########
@@ -104,9 +96,8 @@ class ResourceClaim : public std::enable_shared_from_this<ResourceClaim> {
   }
 
  protected:
-  std::atomic<bool> deleted_;
   // Full path to the content
-  std::string _contentFullPath;
+  const Path _contentFullPath;

Review comment:
       In this case I leave the decision to your taste.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #857: MINIFICPP-1309 - RAII based resourceClaim management

Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on a change in pull request #857:
URL: https://github.com/apache/nifi-minifi-cpp/pull/857#discussion_r480899565



##########
File path: libminifi/include/ResourceClaim.h
##########
@@ -45,8 +45,10 @@ extern std::string default_directory_path;
 extern void setDefaultDirectory(std::string);
 
 // ResourceClaim Class
-class ResourceClaim : public std::enable_shared_from_this<ResourceClaim> {
+class ResourceClaim {
  public:
+  // the type which uniquely represents the resource for the owning manager
+  using Path = std::string;

Review comment:
       changed the ctor
   changing ResourceClaim into a class template incurs quite the change in the codebase, I didn't intend to make ResourceClaim generic, but to separate out the implementation of the "Path", so we could later maybe use Identifier or other stuff, but it's doable to make it generic, would you like me to do it?




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #857: MINIFICPP-1309 - RAII based resourceClaim management

Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on a change in pull request #857:
URL: https://github.com/apache/nifi-minifi-cpp/pull/857#discussion_r464922029



##########
File path: libminifi/test/persistence-tests/PersistenceTests.cpp
##########
@@ -257,8 +258,26 @@ TEST_CASE("Persisted flowFiles are updated on modification", "[TestP1]") {
   config->set(minifi::Configure::nifi_flowfile_repository_directory_default, utils::file::FileUtils::concat_path(dir, "flowfile_repository"));
 
   std::shared_ptr<core::Repository> prov_repo = std::make_shared<TestRepository>();
-  std::shared_ptr<core::repository::FlowFileRepository> ff_repository = std::make_shared<core::repository::FlowFileRepository>("flowFileRepository");
-  std::shared_ptr<core::ContentRepository> content_repo = std::make_shared<core::repository::FileSystemRepository>();
+  std::shared_ptr<core::Repository> ff_repository;
+  std::shared_ptr<core::ContentRepository> content_repo;
+  SECTION("FlowFileRepository"){
+    ff_repository = std::make_shared<core::repository::FlowFileRepository>("flowFileRepository");
+    SECTION("VolatileContentRepository"){
+      content_repo = std::make_shared<core::repository::VolatileContentRepository>();
+    }
+    SECTION("FileSystemContenRepository"){
+      content_repo = std::make_shared<core::repository::FileSystemRepository>();
+    }
+  }
+  SECTION("VolatileFlowFileRepository"){
+    ff_repository = std::make_shared<core::repository::FlowFileRepository>("volatileFlowFileRepository");

Review comment:
       indeed it should




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #857: MINIFICPP-1309 - RAII based resourceClaim management

Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #857:
URL: https://github.com/apache/nifi-minifi-cpp/pull/857#discussion_r481044401



##########
File path: libminifi/include/ResourceClaim.h
##########
@@ -104,9 +96,8 @@ class ResourceClaim : public std::enable_shared_from_this<ResourceClaim> {
   }
 
  protected:
-  std::atomic<bool> deleted_;
   // Full path to the content
-  std::string _contentFullPath;
+  const Path _contentFullPath;

Review comment:
       I'm not sure `const` is a good idea here. It disables copy/move assignment, but not construction. 
   
   More details: https://abseil.io/tips/177
   I tend to generalize these guidelines to "generally avoid `const` or reference non-static data members".




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi-minifi-cpp] szaszm commented on a change in pull request #857: MINIFICPP-1309 - RAII based resourceClaim management

Posted by GitBox <gi...@apache.org>.
szaszm commented on a change in pull request #857:
URL: https://github.com/apache/nifi-minifi-cpp/pull/857#discussion_r480027949



##########
File path: libminifi/include/ResourceClaim.h
##########
@@ -45,8 +45,10 @@ extern std::string default_directory_path;
 extern void setDefaultDirectory(std::string);
 
 // ResourceClaim Class
-class ResourceClaim : public std::enable_shared_from_this<ResourceClaim> {
+class ResourceClaim {
  public:
+  // the type which uniquely represents the resource for the owning manager
+  using Path = std::string;

Review comment:
       Shouldn't this be a class template parameter? 
   
   The two parameter ctor looks like it should use this type alias for its first `path` parameter.

##########
File path: libminifi/src/core/ProcessSession.cpp
##########
@@ -939,9 +939,7 @@ void ProcessSession::persistFlowFilesBeforeTransfer(
         if (ff->isStored() && flowFileRepo->Delete(ff->getUUIDStr())) {
           // original must be non-null since this flowFile is already stored in the repos ->
           // must have come from a session->get()
-          auto claim = original->getResourceClaim();
-          // decrement on behalf of the persisted-instance-to-be-deleted
-          if (claim) claim->decreaseFlowFileRecordOwnedCount();

Review comment:
       Could you explain this change? Why is it no longer necessary to decrease the refcount here?




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #857: MINIFICPP-1309 - RAII based resourceClaim management

Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on a change in pull request #857:
URL: https://github.com/apache/nifi-minifi-cpp/pull/857#discussion_r480886774



##########
File path: libminifi/src/core/ProcessSession.cpp
##########
@@ -939,9 +939,7 @@ void ProcessSession::persistFlowFilesBeforeTransfer(
         if (ff->isStored() && flowFileRepo->Delete(ff->getUUIDStr())) {
           // original must be non-null since this flowFile is already stored in the repos ->
           // must have come from a session->get()
-          auto claim = original->getResourceClaim();
-          // decrement on behalf of the persisted-instance-to-be-deleted
-          if (claim) claim->decreaseFlowFileRecordOwnedCount();

Review comment:
       since `flowFileRepo->Delete` is async, the deletion from the content repository has to be the responsibility of the FlowFileRepository (as was before only with `removeIfOrphaned`), but as we would like to handle the removal of content automatically based on the refCount, that method has been removed, so now a `decreaseFlowFileRecordOwnedCount` will trigger a remove if the refCount goes to zero




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi-minifi-cpp] adamdebreceni commented on pull request #857: MINIFICPP-1309 - RAII based resourceClaim management

Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on pull request #857:
URL: https://github.com/apache/nifi-minifi-cpp/pull/857#issuecomment-669821844


   Indeed I don't remember seeing `RepoTests` fail, although on my fork's CI run it did not fail


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #857: MINIFICPP-1309 - RAII based resourceClaim management

Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on a change in pull request #857:
URL: https://github.com/apache/nifi-minifi-cpp/pull/857#discussion_r466275322



##########
File path: libminifi/test/persistence-tests/PersistenceTests.cpp
##########
@@ -257,8 +258,26 @@ TEST_CASE("Persisted flowFiles are updated on modification", "[TestP1]") {
   config->set(minifi::Configure::nifi_flowfile_repository_directory_default, utils::file::FileUtils::concat_path(dir, "flowfile_repository"));
 
   std::shared_ptr<core::Repository> prov_repo = std::make_shared<TestRepository>();
-  std::shared_ptr<core::repository::FlowFileRepository> ff_repository = std::make_shared<core::repository::FlowFileRepository>("flowFileRepository");
-  std::shared_ptr<core::ContentRepository> content_repo = std::make_shared<core::repository::FileSystemRepository>();
+  std::shared_ptr<core::Repository> ff_repository;
+  std::shared_ptr<core::ContentRepository> content_repo;
+  SECTION("FlowFileRepository"){
+    ff_repository = std::make_shared<core::repository::FlowFileRepository>("flowFileRepository");
+    SECTION("VolatileContentRepository"){
+      content_repo = std::make_shared<core::repository::VolatileContentRepository>();
+    }
+    SECTION("FileSystemContenRepository"){

Review comment:
       done




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi-minifi-cpp] adamdebreceni commented on a change in pull request #857: MINIFICPP-1309 - RAII based resourceClaim management

Posted by GitBox <gi...@apache.org>.
adamdebreceni commented on a change in pull request #857:
URL: https://github.com/apache/nifi-minifi-cpp/pull/857#discussion_r481055566



##########
File path: libminifi/include/ResourceClaim.h
##########
@@ -104,9 +96,8 @@ class ResourceClaim : public std::enable_shared_from_this<ResourceClaim> {
   }
 
  protected:
-  std::atomic<bool> deleted_;
   // Full path to the content
-  std::string _contentFullPath;
+  const Path _contentFullPath;

Review comment:
       a few lines below this we have the copy ctor and the copy assignment declared but undefined and the comment:
   
   > // Prevent default copy constructor and assignment operation
   // Only support pass by reference or pointer
   
   so I think based on this, ResourceClaim was intended to be a `immutable type` of which, the linked resource claims the following:
   
   > There is one useful-but-unusual design that may mandate const members: intentionally immutable types. Instances of such a type are immutable after construction: no mutating methods, no assignment operators. 
   
   of course this might not mean anything, I can remove the `const`, should I do it?




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [nifi-minifi-cpp] szaszm commented on pull request #857: MINIFICPP-1309 - RAII based resourceClaim management

Posted by GitBox <gi...@apache.org>.
szaszm commented on pull request #857:
URL: https://github.com/apache/nifi-minifi-cpp/pull/857#issuecomment-683763463


   :eyes: 


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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org