fixup: review input and bugfixes

kgiusti · Jun 6, 2024 · 1454bb8 · 1454bb8
1 parent 9294e67
commit 1454bb8
Show file tree

Hide file tree

Showing 6 changed files with 17 additions and 12 deletions.
diff --git a/include/qpid/dispatch/log.h b/include/qpid/dispatch/log.h
@@ -56,7 +56,7 @@ typedef enum {
     LOG_HTTP_ADAPTOR,
     LOG_FLOW_LOG,
     LOG_ADDRESS_WATCH,
-    LOG_HTTP_OBSERVER,
+    LOG_HTTP1_OBSERVER,
     LOG_DEFAULT
 } qd_log_module_t;
 

diff --git a/python/skupper_router/management/skrouter.json b/python/skupper_router/management/skrouter.json
@@ -1366,7 +1366,7 @@
                         "HTTP_ADAPTOR",
                         "FLOW_LOG",
                         "ADDRESS_WATCH",
-                        "HTTP_OBSERVER",
+                        "HTTP1_OBSERVER",
                         "DEFAULT"
                     ],
                     "required": true,

diff --git a/src/log.c b/src/log.c
@@ -58,7 +58,7 @@ int qd_log_max_len(void)
 const char *log_module_names[] = {"ROUTER",      "ROUTER_CORE",  "ROUTER_HELLO", "ROUTER_LS",     "ROUTER_MA",
                                   "MESSAGE",     "SERVER",       "AGENT",        "CONTAINER",     "ERROR",
                                   "POLICY",      "HTTP",         "CONN_MGR",     "PYTHON",        "PROTOCOL",
-                                  "TCP_ADAPTOR", "HTTP_ADAPTOR", "FLOW_LOG",     "ADDRESS_WATCH", "HTTP_OBSERVER",
+                                  "TCP_ADAPTOR", "HTTP_ADAPTOR", "FLOW_LOG",     "ADDRESS_WATCH", "HTTP1_OBSERVER",
                                   "DEFAULT"};
 
 typedef struct qd_log_entry_t qd_log_entry_t;

diff --git a/src/observers/http1/http1_observer.c b/src/observers/http1/http1_observer.c
@@ -45,7 +45,7 @@ static int rx_request(qd_http1_decoder_connection_t *hconn, const char *method,
     qdpo_transport_handle_t *th = (qdpo_transport_handle_t *) qd_http1_decoder_connection_get_context(hconn);
     assert(th);
 
-    qd_log(LOG_HTTP_OBSERVER, QD_LOG_DEBUG, "[C%" PRIu64 "] HTTP/1.1 observer rx_request(method=%s target=%s)", th->conn_id, method, target);
+    qd_log(LOG_HTTP1_OBSERVER, QD_LOG_DEBUG, "[C%" PRIu64 "] HTTP/1.1 observer rx_request(method=%s target=%s)", th->conn_id, method, target);
 
     http1_request_state_t *hreq = new_http1_request_state_t();
     ZERO(hreq);
@@ -65,7 +65,7 @@ static int rx_response(qd_http1_decoder_connection_t *hconn, uintptr_t request_c
     qdpo_transport_handle_t *th = (qdpo_transport_handle_t *) qd_http1_decoder_connection_get_context(hconn);
     assert(th);
 
-    qd_log(LOG_HTTP_OBSERVER, QD_LOG_DEBUG, "[C%" PRIu64 "] HTTP/1.1 observer rx_response(status=%d reason=%s)", th->conn_id, status_code, reason_phrase);
+    qd_log(LOG_HTTP1_OBSERVER, QD_LOG_DEBUG, "[C%" PRIu64 "] HTTP/1.1 observer rx_response(status=%d reason=%s)", th->conn_id, status_code, reason_phrase);
 
     http1_request_state_t *hreq = (http1_request_state_t *) request_context;
     assert(hreq);
@@ -92,7 +92,7 @@ static int transaction_complete(qd_http1_decoder_connection_t *hconn, uintptr_t
     qdpo_transport_handle_t *th = (qdpo_transport_handle_t *) qd_http1_decoder_connection_get_context(hconn);
     assert(th);
 
-    qd_log(LOG_HTTP_OBSERVER, QD_LOG_DEBUG, "[C%" PRIu64 "] HTTP/1.1 observer transaction complete", th->conn_id);
+    qd_log(LOG_HTTP1_OBSERVER, QD_LOG_DEBUG, "[C%" PRIu64 "] HTTP/1.1 observer transaction complete", th->conn_id);
 
     http1_request_state_t *hreq = (http1_request_state_t *) request_context;
     assert(hreq);
@@ -113,10 +113,10 @@ static void protocol_error(qd_http1_decoder_connection_t *hconn, const char *rea
     // carrying HTTP/1.x data.
 
     if (DEQ_SIZE(th->http1.requests)) {
-        qd_log(LOG_HTTP_OBSERVER, QD_LOG_ERROR,
+        qd_log(LOG_HTTP1_OBSERVER, QD_LOG_ERROR,
                "[C%" PRIu64 "] HTTP/1.1 observer disabled due to protocol error: %s", th->conn_id, reason);
     } else {
-        qd_log(LOG_HTTP_OBSERVER, QD_LOG_DEBUG, "[C%" PRIu64 "] HTTP/1.1 observer protocol error: %s", th->conn_id, reason);
+        qd_log(LOG_HTTP1_OBSERVER, QD_LOG_DEBUG, "[C%" PRIu64 "] HTTP/1.1 observer protocol error: %s", th->conn_id, reason);
     }
 
     // An error code will be returned to the http1_observe() call and this observer will be cleaned up there.
@@ -148,7 +148,7 @@ static void http1_observe(qdpo_transport_handle_t *th, bool from_client, const u
 
 void qdpo_http1_init(qdpo_transport_handle_t *th)
 {
-    qd_log(LOG_HTTP_OBSERVER, QD_LOG_DEBUG, "[C%" PRIu64 "] HTTP/1.1 observer initialized", th->conn_id);
+    qd_log(LOG_HTTP1_OBSERVER, QD_LOG_DEBUG, "[C%" PRIu64 "] HTTP/1.1 observer initialized", th->conn_id);
 
     th->protocol  = QD_PROTOCOL_HTTP1;
     th->observe   = http1_observe;
@@ -166,7 +166,7 @@ void qdpo_http1_init(qdpo_transport_handle_t *th)
 
 void qdpo_http1_final(qdpo_transport_handle_t *th)
 {
-    qd_log(LOG_HTTP_OBSERVER, QD_LOG_DEBUG, "[C%" PRIu64 "] HTTP/1.1 observer finalized", th->conn_id);
+    qd_log(LOG_HTTP1_OBSERVER, QD_LOG_DEBUG, "[C%" PRIu64 "] HTTP/1.1 observer finalized", th->conn_id);
 
     if (th->observe) {
         th->observe = 0;

diff --git a/tests/system_test.py b/tests/system_test.py
@@ -522,7 +522,9 @@ def __init__(self, port, name=None, expect=Process.RUNNING, **kwargs):
         name = name or "http.server"
         kwargs.setdefault('stdin', subprocess.DEVNULL)  # no input accepted
         kwargs.setdefault('directory', os.path.join(current_dir, "http1-data"))
-        kwargs.setdefault('protocol', "HTTP/1.1")
+        if sys.version_info >= (3, 11):
+            # --protocol added in python3.11
+            kwargs.setdefault('protocol', "HTTP/1.1")
         args = [sys.executable,
                 "-m", "http.server",
                 "-d", kwargs['directory'],

diff --git a/tests/system_tests_http1_observer.py b/tests/system_tests_http1_observer.py
@@ -22,6 +22,7 @@
 #
 
 import os
+import sys
 
 from system_test import TestCase, unittest, main_module, Qdrouterd
 from system_test import curl_available, run_curl
@@ -85,7 +86,7 @@ def router(cls, name, listener_port, server_port, extra_config=None):
                      'includeTimestamp': 'false',
                      'includeSource': 'false',
                      'outputFile': os.path.abspath(f"{name}-flow.log")}),
-            ('log', {'module': 'HTTP_OBSERVER', 'enable': 'debug+'}),
+            ('log', {'module': 'HTTP1_OBSERVER', 'enable': 'debug+'}),
             ('log', {'module': 'FLOW_LOG', 'enable': 'debug+'})
         ]
 
@@ -108,6 +109,7 @@ def setUpClass(cls):
         cls.http1_port = cls.tester.get_port()
         cls.http1_server = cls.tester.cleanup(Http1Server(cls.http1_port))
 
+    @unittest.skipUnless(sys.version_info >= (3, 11), "Requires HTTP/1.1 support")
     def test_01_get(self):
         """
         Simple pipelined GET request.
@@ -163,6 +165,7 @@ def test_01_get(self):
 
         self.assertEqual(len(pages), matches, f"unexpected results {results}")
 
+    @unittest.skipUnless(sys.version_info >= (3, 11), "Requires HTTP/1.1 support")
     def test_02_post(self):
         """
         Simple POST request (chunked). Uses the Http server from the Python