Fix request multipart logging when only 1 part is present
authorJohn Dennis <jdennis@redhat.com>
Tue, 27 Jan 2015 16:53:31 +0000 (11:53 -0500)
committerSimo Sorce <simo@redhat.com>
Tue, 27 Jan 2015 21:40:46 +0000 (16:40 -0500)
Test to see if the request parameter value is a cherrypy Part
class. This was already being done for the case where the value was a
list, but it was omitted for single values. Logic was combined into new
local function print_param().

Changed the test for the class back to using

    if isinstance(item, cherrypy._cpreqbody.Part):

instead of:

    if getattr(item, "part_class", None):

because using isinstance() clearly indicates what is being done. The
use of getattr() was introduced to prevent a pylint warning concering
use of protected values. The getattr() hack is confusing and proably
not robust if the class implementation changes. The patch now disables
this warning. I cannot explain why cherrypy marks these modules as
protected when clearly one has to utilize them and they are documented
in the cherrypy API doc. Disabling the warning seems the cleanest and
most robust approach.

Signed-off-by: John Dennis <jdennis@redhat.com>
Reviewed-by: Simo Sorce <simo@redhat.com>
ipsilon/util/log.py

index 0a7a9df..6ccfc42 100644 (file)
@@ -136,6 +136,21 @@ def log_request_response():
         f.close()
         return string
 
+    def print_param(name, value):
+        f = cStringIO.StringIO()
+
+        # Might be a multipart Part object, if so format it
+        if isinstance(value, cherrypy._cpreqbody.Part):  # pylint:disable=W0212
+            f.write(indent_text("%s:\n" % (name)))
+            f.write(indent_text(print_part(value), 1))
+        else:
+            # Not a mulitpart, just write it as a string
+            f.write(indent_text("%s: %s\n" % (name, value)))
+
+        string = f.getvalue()
+        f.close()
+        return string
+
     def collapse_body(body):
         '''The cherrypy response body can be:
 
@@ -197,17 +212,10 @@ def log_request_response():
             # Multi-valued paramater is in a list
             if isinstance(value, list):
                 for i, item in enumerate(value):
-                    # Might be a multipart Part object, if so format it
-                    if getattr(item, "part_class", None):
-                        f.write(indent_text("%s[%s]:\n" % (name, i), 2))
-                        f.write(indent_text(print_part(item), 3))
-                    else:
-                        # Not a mulitpart, just write it as a string
-                        f.write(indent_text("%s[%s]: %s\n" %
-                                            (name, i, item), 2))
+                    f.write(indent_text(print_param("%s[%d]" % (name, i),
+                                                    item), 2))
             else:
-                # Just a string value
-                f.write(indent_text("%s: %s\n" % (name, value), 2))
+                f.write(indent_text(print_param(name, value), 2))
 
     # If the body is multipart format each of the parts
     if request.body.parts: