[GitHub] [nifi-minifi-cpp] szaszm commented on a diff in pull request #1616: MINIFICPP-2173 Fix MergeContent error handling issues

["szaszm (via GitHub)" <gi...@apache.org>]

szaszm commented on code in PR #1616:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1616#discussion_r1279544211


##########
extensions/libarchive/BinFiles.h:
##########
@@ -284,19 +285,24 @@ class BinFiles : public core::Processor {
 
  protected:
   // Allows general pre-processing of a flow file before it is offered to a bin. This is called before getGroupId().
-  virtual void preprocessFlowFile(core::ProcessContext *context, core::ProcessSession *session, const std::shared_ptr<core::FlowFile>& flow);
+  virtual void preprocessFlowFile(const std::shared_ptr<core::FlowFile>& flow);
   // Returns a group ID representing a bin. This allows flow files to be binned into like groups
-  virtual std::string getGroupId(core::ProcessContext* /*context*/, const std::shared_ptr<core::FlowFile>& /*flow*/) {
+  virtual std::string getGroupId(const std::shared_ptr<core::FlowFile>& /*flow*/) {
     return "";
   }
   // Processes a single bin.
-  virtual bool processBin(core::ProcessContext* /*context*/, core::ProcessSession* /*session*/, std::unique_ptr<Bin>& /*bin*/) {
+  virtual bool processBin(core::ProcessSession& /*session*/, std::unique_ptr<Bin>& /*bin*/) {
     return false;
   }
   // transfer flows to failure in bin
-  static void transferFlowsToFail(core::ProcessContext *context, core::ProcessSession *session, std::unique_ptr<Bin> &bin);
+  static void transferFlowsToFail(core::ProcessSession &session, std::unique_ptr<Bin> &bin);
   // moves owned flows to session
-  static void addFlowsToSession(core::ProcessContext *context, core::ProcessSession *session, std::unique_ptr<Bin> &bin);
+  static void addFlowsToSession(core::ProcessSession &session, std::unique_ptr<Bin> &bin);
+
+  bool resurrectFlowFiles(core::ProcessSession &session);
+  void assumeOwnerShipOfNextBatch(core::ProcessSession &session);
+  std::deque<std::unique_ptr<Bin>> gatherReadyBins(core::ProcessContext &context);
+  void processReadyBins(std::deque<std::unique_ptr<Bin>> ready_bins, core::ProcessSession &session);

Review Comment:
   Could you write a comment explaining the inner working of BinFiles and MergeContent, as a class or file level comment? I guess you had to understand it to some degree to make this change, and I would appreciate sharing this knowledge.
   
   If that's too big of an ask, could you document just the concepts related to this patch?
   
   Some words worth explaining IMO:
   - resurrect (flow file)
   - batch
   - bin
   - ready bin
   - preprocess (flow file)
   - group
   - bundle
   - entry
   
   Or just the ones related to this PR:
   - resurrect (flow file)
   - batch (how is it different from bin, group, bundle, etc.)
   - ready bins
   
   This would also help with reviewing.



-- 
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-unsubscribe@nifi.apache.org

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