[GitHub] [trafficserver] shukitchan commented on a change in pull request #7233: [multiplexer] option to skip post/put requests

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

shukitchan commented on a change in pull request #7233:
URL: https://github.com/apache/trafficserver/pull/7233#discussion_r498571698



##########
File path: plugins/multiplexer/ats-multiplexer.cc
##########
@@ -77,8 +80,19 @@ TSRemapNewInstance(int argc, char **argv, void **i, char *, int)
   Instance *instance = new Instance;
 
   if (argc > 2) {
-    std::copy(argv + 2, argv + argc, std::back_inserter(instance->origins));
+    std::copy_if(argv + 2, argv + argc, std::back_inserter(instance->origins), [&](std::string s) {
+      std::regex srgx("proxy\\.config\\.multiplexer\\.skip\\_post\\_put=(\\d)");

Review comment:
       why do we need a regex to match the parameter? 
   perhaps we can be a bit simpler with just a string match and expand/enhance on that if we have many more parameters later on?

##########
File path: plugins/multiplexer/ats-multiplexer.cc
##########
@@ -131,19 +145,24 @@ DoRemap(const Instance &i, TSHttpTxn t)
 
   TSDebug(PLUGIN_TAG, "Method is %s.", std::string(method, length).c_str());
 
-  if (length == TS_HTTP_LEN_POST && memcmp(TS_HTTP_METHOD_POST, method, TS_HTTP_LEN_POST) == 0) {
-    const TSVConn vconnection = TSTransformCreate(handlePost, t);
-    assert(vconnection != nullptr);
-    TSContDataSet(vconnection, new PostState(requests));
-    assert(requests.empty());
-    TSHttpTxnHookAdd(t, TS_HTTP_REQUEST_TRANSFORM_HOOK, vconnection);
+  if (skip_post_put && ((length == TS_HTTP_LEN_POST && memcmp(TS_HTTP_METHOD_POST, method, TS_HTTP_LEN_POST) == 0) ||
+                        (length == TS_HTTP_LEN_PUT && memcmp(TS_HTTP_METHOD_PUT, method, TS_HTTP_LEN_PUT) == 0))) {
+    TSHandleMLocRelease(buffer, TS_NULL_MLOC, location);
   } else {
-    dispatch(requests, timeout);
-  }
+    if (length == TS_HTTP_LEN_POST && memcmp(TS_HTTP_METHOD_POST, method, TS_HTTP_LEN_POST) == 0) {

Review comment:
       perhaps it is better to do generateRequests()  + requests assert + field creation inside this block instead of outside? 

##########
File path: plugins/multiplexer/ats-multiplexer.cc
##########
@@ -39,6 +41,7 @@
 const size_t DEFAULT_TIMEOUT = 1000000000000;
 
 Statistics statistics;
+bool skip_post_put = false;

Review comment:
       Instead of a variable here. Should we keep all the parameters in one place under the Instance object? 
   Perhaps we can add another variable to the Instance object for this flag insead? 




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