Re: [PR] MINIFICPP-2259 ProcessContext::getProperty should operate on raw poin… [nifi-minifi-cpp]
szaszm closed pull request #1707: MINIFICPP-2259 ProcessContext::getProperty should operate on raw poin… URL: https://github.com/apache/nifi-minifi-cpp/pull/1707 -- 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
Re: [PR] MINIFICPP-2259 ProcessContext::getProperty should operate on raw poin… [nifi-minifi-cpp]
martinzink commented on code in PR #1707: URL: https://github.com/apache/nifi-minifi-cpp/pull/1707#discussion_r1447266560 ## libminifi/src/core/ProcessSession.cpp: ## @@ -110,36 +109,36 @@ std::shared_ptr ProcessSession::create(const std::shared_ptrsetLineageStartDate(parent->getlineageStartDate()); record->setLineageIdentifiers(parent->getlineageIdentifiers()); -parent->getlineageIdentifiers().push_back(parent->getUUID()); +record->getlineageIdentifiers().push_back(parent->getUUID()); Review Comment: yeah -- 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
Re: [PR] MINIFICPP-2259 ProcessContext::getProperty should operate on raw poin… [nifi-minifi-cpp]
fgerlits commented on code in PR #1707: URL: https://github.com/apache/nifi-minifi-cpp/pull/1707#discussion_r1447129423 ## libminifi/src/core/ProcessSession.cpp: ## @@ -110,36 +109,36 @@ std::shared_ptr ProcessSession::create(const std::shared_ptrsetLineageStartDate(parent->getlineageStartDate()); record->setLineageIdentifiers(parent->getlineageIdentifiers()); -parent->getlineageIdentifiers().push_back(parent->getUUID()); +record->getlineageIdentifiers().push_back(parent->getUUID()); Review Comment: was this a bug? -- 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
Re: [PR] MINIFICPP-2259 ProcessContext::getProperty should operate on raw poin… [nifi-minifi-cpp]
martinzink commented on code in PR #1707: URL: https://github.com/apache/nifi-minifi-cpp/pull/1707#discussion_r1431353285 ## extensions/aws/processors/FetchS3Object.h: ## @@ -46,6 +46,7 @@ class FetchS3Object : public S3Processor { .supportsExpressionLanguage(true) .build(); EXTENSIONAPI static constexpr auto Version = core::PropertyDefinitionBuilder<>::createProperty("Version") + .withDescription("The Version of the Object to download") Review Comment: good catch fixed it in https://github.com/apache/nifi-minifi-cpp/pull/1707/commits/d64e7f890b82be04ec5a75bc484a1ebcbb001f32 ## libminifi/src/core/ProcessSession.cpp: ## @@ -199,21 +198,19 @@ void ProcessSession::remove(const std::shared_ptr ) { flow->setDeleted(true); deleted_flowfiles_.push_back(flow); std::string reason = process_context_->getProcessorNode()->getName() + " drop flow record " + flow->getUUIDStr(); - provenance_report_->drop(flow, reason); + provenance_report_->drop(*flow, reason); } -void ProcessSession::putAttribute(const std::shared_ptr& flow, std::string_view key, const std::string& value) { - flow->setAttribute(key, value); - std::stringstream details; - details << process_context_->getProcessorNode()->getName() << " modify flow record " << flow->getUUIDStr() << " attribute " << key << ":" << value; - provenance_report_->modifyAttributes(flow, details.str()); +void ProcessSession::putAttribute(core::FlowFile& flow_file, std::string_view key, const std::string& value) { + flow_file.setAttribute(key, value); + std::string details = fmt::format("{} modify flow record {} attribute {} : {}", process_context_->getProcessorNode()->getName(), flow_file.getUUIDStr(), key, value); Review Comment: good catch fixed it in https://github.com/apache/nifi-minifi-cpp/pull/1707/commits/d64e7f890b82be04ec5a75bc484a1ebcbb001f32 -- 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
Re: [PR] MINIFICPP-2259 ProcessContext::getProperty should operate on raw poin… [nifi-minifi-cpp]
szaszm commented on code in PR #1707: URL: https://github.com/apache/nifi-minifi-cpp/pull/1707#discussion_r1425368129 ## extensions/aws/processors/FetchS3Object.h: ## @@ -46,6 +46,7 @@ class FetchS3Object : public S3Processor { .supportsExpressionLanguage(true) .build(); EXTENSIONAPI static constexpr auto Version = core::PropertyDefinitionBuilder<>::createProperty("Version") + .withDescription("The Version of the Object to download") Review Comment: duplicated line ## libminifi/src/core/ProcessSession.cpp: ## @@ -199,21 +198,19 @@ void ProcessSession::remove(const std::shared_ptr ) { flow->setDeleted(true); deleted_flowfiles_.push_back(flow); std::string reason = process_context_->getProcessorNode()->getName() + " drop flow record " + flow->getUUIDStr(); - provenance_report_->drop(flow, reason); + provenance_report_->drop(*flow, reason); } -void ProcessSession::putAttribute(const std::shared_ptr& flow, std::string_view key, const std::string& value) { - flow->setAttribute(key, value); - std::stringstream details; - details << process_context_->getProcessorNode()->getName() << " modify flow record " << flow->getUUIDStr() << " attribute " << key << ":" << value; - provenance_report_->modifyAttributes(flow, details.str()); +void ProcessSession::putAttribute(core::FlowFile& flow_file, std::string_view key, const std::string& value) { + flow_file.setAttribute(key, value); + std::string details = fmt::format("{} modify flow record {} attribute {} : {}", process_context_->getProcessorNode()->getName(), flow_file.getUUIDStr(), key, value); Review Comment: there was no space between the key and the value before the change ```suggestion std::string details = fmt::format("{} modify flow record {} attribute {}:{}", process_context_->getProcessorNode()->getName(), flow_file.getUUIDStr(), key, value); ``` -- 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
Re: [PR] MINIFICPP-2259 ProcessContext::getProperty should operate on raw poin… [nifi-minifi-cpp]
martinzink commented on PR #1707: URL: https://github.com/apache/nifi-minifi-cpp/pull/1707#issuecomment-1851594569 Similarly to https://github.com/apache/nifi-minifi-cpp/pull/1693 I'd ask you to be a bit more lenient with the clang tidy results. I'd rather not refactor the whole codebase in this PR -- 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