[GitHub] [nifi-minifi-cpp] fgerlits commented on a diff in pull request #1344: MINIFICPP-1845 - Add c2 request compression option

[GitBox <gi...@apache.org>]

fgerlits commented on code in PR #1344:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1344#discussion_r891129193


##########
extensions/http-curl/protocols/RESTSender.cpp:
##########
@@ -124,10 +133,30 @@ C2Payload RESTSender::sendPayload(const std::string url, const Direction directi
     } else {
       auto data_input = std::make_unique<utils::ByteInputCallback>();
       auto data_cb = std::make_unique<utils::HTTPUploadCallback>();
-      data_input->write(data.value_or(""));
+      if (data && req_encoding_ == RequestEncoding::Gzip) {
+        io::BufferStream compressed_payload;
+        bool compression_success = [&] {
+          io::ZlibCompressStream compressor(gsl::make_not_null(&compressed_payload), io::ZlibCompressionFormat::GZIP, Z_BEST_COMPRESSION);
+          auto ret = compressor.write(gsl::span<const char>(data.value()).as_span<const std::byte>());
+          if (ret != data->length()) {
+            return false;

Review Comment:
   we don't need `compressor.close()` in this case?



##########
extensions/http-curl/protocols/RESTSender.cpp:
##########
@@ -52,6 +49,18 @@ void RESTSender::initialize(core::controller::ControllerServiceProvider* control
         ssl_context_service_ = std::static_pointer_cast<minifi::controllers::SSLContextService>(service);
       }
     }
+    if (auto req_encoding_str = configure->get(Configuration::nifi_c2_rest_request_encoding)) {
+      if (auto req_encoding = RequestEncoding::parse(req_encoding_str->c_str(), RequestEncoding{})) {

Review Comment:
   it might be more user-friendly to do this in a case-insensitive way



##########
conf/minifi.properties:
##########
@@ -102,11 +102,16 @@ nifi.c2.root.class.definitions.metrics.metrics.processorMetrics.classes=GetFileM
 ## HeartbeatLogger logs the heartbeats on TRACE for debugging.
 #nifi.c2.agent.heartbeat.reporter.classes=HeartbeatLogger
 
+# specify encoding strategy for c2 requests (gzip, none)
+#nifi.c2.rest.request.encoding=none
+
 ## enable the controller socket provider on port 9998
 ## off by default. C2 must be enabled to support these
 #controller.socket.host=localhost
 #controller.socket.port=9998
 
+## specify the destination of c2 directed assets
+#nifi.asset.directory=${MINIFI_HOME}/asset

Review Comment:
   Should the two `asset.directory`-related changes be in this PR?  They don't seem to be related to C2 request compression.



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