nifi-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [nifi-minifi-cpp] arpadboda commented on a change in pull request #775: MINIFICPP-965 Add other supported relationships to InvokeHTTP processor.
Date Mon, 04 May 2020 10:13:22 GMT

arpadboda commented on a change in pull request #775:
URL: https://github.com/apache/nifi-minifi-cpp/pull/775#discussion_r419326908



##########
File path: extensions/http-curl/client/HTTPClient.cpp
##########
@@ -296,12 +296,13 @@ bool HTTPClient::submit() {
   }
   curl_easy_setopt(http_session_, CURLOPT_HEADERFUNCTION, &utils::HTTPHeaderResponse::receive_headers);
   curl_easy_setopt(http_session_, CURLOPT_HEADERDATA, static_cast<void*>(&header_response_));
-  if (keep_alive_probe_ > 0){
-    logger_->log_debug("Setting keep alive to %d",keep_alive_probe_);
+  if (keep_alive_probe_.count() > 0) {
+    const auto keepAlive = std::chrono::duration_cast<std::chrono::duration<uint64_t>>(keep_alive_probe_);

Review comment:
       I think casting to std::chrono::seconds (which is actually duration<uint64_t>)
is a bit more talkative.

##########
File path: extensions/http-curl/client/HTTPClient.h
##########
@@ -124,11 +135,11 @@ class HTTPClient : public BaseHTTPClient, public core::Connectable {
 
   bool setMinimumSSLVersion(SSLVersion minimum_version) override;
 
-  void setKeepAliveProbe(long probe){
+  void setKeepAliveProbe(std::chrono::milliseconds probe){
     keep_alive_probe_ = probe;
   }
 
-  void setKeepAliveIdle(long idle){

Review comment:
       Could we keep this as deprecated functions as well?

##########
File path: extensions/http-curl/tests/HTTPHandlers.h
##########
@@ -445,4 +445,49 @@ class HeartbeatHandler : public CivetHandler {
   bool isSecure;
 };
 
+class InvokeHTTPResponseOKHandler : public CivetHandler {
+public:
+  bool handlePost(CivetServer *, struct mg_connection *conn) {
+    std::stringstream headers;
+    headers << "HTTP/1.1 201 OK\r\nContent-Type: text/plain\r\nContent-Length: 0\r\nConnection:
close\r\n\r\n";

Review comment:
       I don't think stringstream makes sense in case we only put one string to it. 
   Can we simply pass the hardcoded cstr to the mg_printf?

##########
File path: extensions/http-curl/tests/HTTPHandlers.h
##########
@@ -445,4 +445,49 @@ class HeartbeatHandler : public CivetHandler {
   bool isSecure;
 };
 
+class InvokeHTTPResponseOKHandler : public CivetHandler {
+public:
+  bool handlePost(CivetServer *, struct mg_connection *conn) {
+    std::stringstream headers;
+    headers << "HTTP/1.1 201 OK\r\nContent-Type: text/plain\r\nContent-Length: 0\r\nConnection:
close\r\n\r\n";
+    mg_printf(conn, headers.str().c_str());
+    return true;
+  }
+};
+
+class InvokeHTTPResponse404Handler : public CivetHandler {
+public:
+  bool handlePost(CivetServer *, struct mg_connection *conn) {
+    std::stringstream headers;
+    headers << "HTTP/1.1 404 Not Found\r\nContent-Type: text/plain\r\nContent-Length:
0\r\nConnection: close\r\n\r\n";
+    mg_printf(conn, headers.str().c_str());
+    return true;
+  }
+};
+
+class InvokeHTTPResponse501Handler : public CivetHandler {
+public:
+  bool handlePost(CivetServer *, struct mg_connection *conn) {
+    std::stringstream headers;
+    headers << "HTTP/1.1 501 Not Implemented\r\nContent-Type: text/plain\r\nContent-Length:
0\r\nConnection: close\r\n\r\n";
+    mg_printf(conn, headers.str().c_str());
+    return true;
+  }
+};
+
+class InvokeHTTPResponseTimeoutHandler : public CivetHandler {
+public:
+    InvokeHTTPResponseTimeoutHandler(std::chrono::milliseconds wait_ms)
+        : wait_(wait_ms) {
+    }
+  bool handlePost(CivetServer *, struct mg_connection *conn) {
+    std::this_thread::sleep_for(wait_);

Review comment:
       Nice solution!

##########
File path: libminifi/test/resources/TestInvokeHTTPPost.yml
##########
@@ -0,0 +1,161 @@
+MiNiFi Config Version: 3
+Flow Controller:
+  name: c++lw
+  comment: Created by MiNiFi C2 Flow Designer
+Core Properties:
+  flow controller graceful shutdown period: 10 sec
+  flow service write delay interval: 500 ms
+  administrative yield duration: 30 sec
+  bored yield duration: 10 millis
+  max concurrent threads: 1
+  variable registry properties: ''
+FlowFile Repository:
+  partitions: 256
+  checkpoint interval: 2 mins
+  always sync: false
+  Swap:
+    threshold: 20000
+    in period: 5 sec
+    in threads: 1
+    out period: 5 sec
+    out threads: 4
+Content Repository:
+  content claim max appendable size: 10 MB
+  content claim max flow files: 100
+  always sync: false
+Provenance Repository:
+  provenance rollover time: 1 min
+  implementation: org.apache.nifi.provenance.MiNiFiPersistentProvenanceRepository
+Component Status Repository:
+  buffer size: 1440
+  snapshot frequency: 1 min
+Security Properties:
+  keystore: ''
+  keystore type: ''
+  keystore password: ''
+  key password: ''
+  truststore: ''
+  truststore type: ''
+  truststore password: ''
+  ssl protocol: ''
+  Sensitive Props:
+    key:
+    algorithm: PBEWITHMD5AND256BITAES-CBC-OPENSSL
+    provider: BC
+Processors:
+- id: 4ed2d51d-076a-49b0-88de-5cf5adf52a7e
+  name: GenerateFlowFile
+  class: org.apache.nifi.minifi.processors.GenerateFlowFile
+  max concurrent tasks: 1
+  scheduling strategy: TIMER_DRIVEN
+  scheduling period: 15000 ms
+  penalization period: 1000 ms
+  yield period: 1000 ms
+  run duration nanos: 0
+  auto-terminated relationships list: []
+  Properties:
+    Batch Size: '1'
+    Data Format: Binary
+    File Size: 1 kB
+    Unique FlowFiles: 'true'
+- id: 1d51724d-dd76-46a0-892d-a7c7408d58dd
+  name: InvokeHTTP
+  class: org.apache.nifi.minifi.processors.InvokeHTTP
+  max concurrent tasks: 1
+  scheduling strategy: TIMER_DRIVEN
+  scheduling period: 1000 ms
+  penalization period: 1000 ms
+  yield period: 1000 ms
+  run duration nanos: 0
+  auto-terminated relationships list: []
+  Properties:
+    Always Output Response: 'false'
+    Connection Timeout: 3 s
+    Content-type: application/octet-stream
+    Disable Peer Verification: 'false'
+    HTTP Method: POST
+    Include Date Header: 'true'
+    Read Timeout: 4 s
+    Remote URL: http://localhost:10033/minifi

Review comment:
       I don't really like using hardcoded magic ports and in my opinion we can avoid this.
   
   Civet server supports starting the webserver on random port (I think we already have some
examples for that usage in our tests), and in queryRootProcessGroup we could override this
property.
   The civet server should be started before this function executes, but I think that's already
done in proper 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



Mime
View raw message