szaszm commented on code in PR #2069:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2069#discussion_r2691028625
##########
libminifi/src/properties/Properties.cpp:
##########
@@ -199,59 +252,65 @@ void PropertiesImpl::loadConfigureFile(const
std::filesystem::path& configuratio
dirty_ = dirty_ || need_to_persist_new_value;
properties_[key] = {persisted_value, value, need_to_persist_new_value};
}
- checksum_calculator_.setFileLocation(properties_file_);
}
std::filesystem::path PropertiesImpl::getFilePath() const {
std::lock_guard<std::mutex> lock(mutex_);
- return properties_file_;
+ return base_properties_file_;
}
bool PropertiesImpl::commitChanges() {
std::lock_guard<std::mutex> lock(mutex_);
if (!dirty_) {
- logger_->log_info("Attempt to persist, but properties are not updated");
+ logger_->log_debug("commitChanges() called, but properties have not
changed, nothing to do");
return true;
}
- std::ifstream file(properties_file_, std::ifstream::in);
+ const auto output_file = (persist_to_ == PersistTo::SingleFile ?
base_properties_file_ : extra_properties_files_dir_name() /
C2PropertiesFileName);
Review Comment:
I'm not sure what would be the best way to decide which file to write
updates to, but it's a bit strange that we hardcode a C2-related filename for
(I'm assuming) property overrides through C2 property update, in a function
whose signature doesn't suggest any inherent relation to C2. Could we maybe
pass in the target filename instead?
##########
conf/minifi-uid.properties.in:
##########
@@ -13,6 +13,10 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+### NOTE: Changes to this file are overwritten on upgrade. Add your custom
settings to new files ###
+### in the conf/minifi-uid.properties.d directory instead. These custom
settings files should have ###
+### a name ending with ".properties", and they are applied in lexicographic
order, after this file. ###
Review Comment:
```suggestion
### a name ending with ".properties", and they are applied in lexicographic
order, after this file. ###
### Example filename: conf/minifi-uid.properties.d/10-use-random.properties
```
##########
conf/minifi-log.properties.in:
##########
@@ -13,6 +13,10 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+### NOTE: Changes to this file are overwritten on upgrade. Add your custom
settings to new files ###
+### in the conf/minifi-log.properties.d directory instead. These custom
settings files should have ###
+### a name ending with ".properties", and they are applied in lexicographic
order, after this file. ###
Review Comment:
I'd add example filenames, to suggest that users follow the typical linux
conventions of two prefix digits
```suggestion
### a name ending with ".properties", and they are applied in lexicographic
order, after this file. ###
### Example filename:
conf/minifi-log.properties.d/90-log-to-stderr.properties
```
##########
conf/minifi.properties.in:
##########
@@ -13,6 +13,10 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+### NOTE: Changes to this file are overwritten on upgrade. Add your custom
settings to new files ###
+### in the conf/minifi.properties.d directory instead. These custom settings
files should have ###
+### a name ending with ".properties", and they are applied in lexicographic
order, after this file. ###
Review Comment:
```suggestion
### a name ending with ".properties", and they are applied in lexicographic
order, after this file. ###
### Example filename: conf/minifi.properties.d/10-c2.properties,
conf/minifi.properties.d/90-repository-tweaks.properties
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
