Update patch set 2

Patch Set 2: Code-Review-1

(7 comments)

Patch-set: 2
Label: Code-Review=-1

ea922e2cc799527f56ad94bff99401eb5ea2e25d

@@ -0,0 +1,123 @@
+{
+  "comments": [
+    {
+      "key": {
+        "uuid": "3a961159_908046c0",
+        "filename": "shade/__init__.py",
+        "patchSetId": 2
+      },
+      "lineNbr": 245,
+      "author": {
+        "id": 1
+      },
+      "writtenOn": "2015-01-08T22:53:38Z",
+      "side": 1,
+      "message": "There is a commented out line here.",
+      "revId": "ea922e2cc799527f56ad94bff99401eb5ea2e25d",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": false
+    },
+    {
+      "key": {
+        "uuid": "3a961159_b08382c9",
+        "filename": "shade/__init__.py",
+        "patchSetId": 2
+      },
+      "lineNbr": 671,
+      "author": {
+        "id": 1
+      },
+      "writtenOn": "2015-01-08T22:53:38Z",
+      "side": 1,
+      "message": "Should this really silently succeed?",
+      "revId": "ea922e2cc799527f56ad94bff99401eb5ea2e25d",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": false
+    },
+    {
+      "key": {
+        "uuid": "3a961159_d06d6e33",
+        "filename": "shade/__init__.py",
+        "patchSetId": 2
+      },
+      "lineNbr": 674,
+      "author": {
+        "id": 1
+      },
+      "writtenOn": "2015-01-08T22:53:38Z",
+      "side": 1,
+      "message": "Should this return the container?",
+      "revId": "ea922e2cc799527f56ad94bff99401eb5ea2e25d",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": false
+    },
+    {
+      "key": {
+        "uuid": "3a961159_f068aa22",
+        "filename": "shade/__init__.py",
+        "patchSetId": 2
+      },
+      "lineNbr": 681,
+      "author": {
+        "id": 1
+      },
+      "writtenOn": "2015-01-08T22:53:38Z",
+      "side": 1,
+      "message": "This is a portability concern, yeah?  We\u0027re depending on the md5sum program.  How about hashlib instead: http://stackoverflow.com/questions/1131220/get-md5-hash-of-big-files-in-python\n\nRead it in, say 4096 or 8192 byte chunks for efficiency and fitting in memory.",
+      "revId": "ea922e2cc799527f56ad94bff99401eb5ea2e25d",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": false
+    },
+    {
+      "key": {
+        "uuid": "3a961159_107456fc",
+        "filename": "shade/__init__.py",
+        "patchSetId": 2
+      },
+      "lineNbr": 687,
+      "author": {
+        "id": 1
+      },
+      "writtenOn": "2015-01-08T22:53:38Z",
+      "side": 1,
+      "message": "I\u0027m not sure what\u0027s going on here -- it looks like this is expecting the 404 not found case to raise this exception.  However, I think any number of conditions could raise the exception, including 5xx codes, and maybe even lack of connectivity.  Maybe we should check the ClientException for a 404 code.  However, you lost that information when masking the exception with OpenStackCloudException.",
+      "revId": "ea922e2cc799527f56ad94bff99401eb5ea2e25d",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": false
+    },
+    {
+      "key": {
+        "uuid": "3a961159_306f9229",
+        "filename": "shade/__init__.py",
+        "patchSetId": 2
+      },
+      "lineNbr": 694,
+      "author": {
+        "id": 1
+      },
+      "writtenOn": "2015-01-08T22:53:38Z",
+      "side": 1,
+      "message": "http://docs.openstack.org/developer/swift/overview_large_objects.html\n\n\"Usually in Swift the ETag is the MD5 sum of the contents of the object\"\n\nDoes it really mean \"always\" instead of \"usually\"?  If so, maybe the docs should be updated.  If not, then this might not work at all with some swift installations.\n\nAlso, that\u0027s really unfortunate because we should not be using md5s for this sort of thing because of the hash collision weakness (a malicious party could create a collision either locally or in the cloud and fool this code).",
+      "revId": "ea922e2cc799527f56ad94bff99401eb5ea2e25d",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": false
+    },
+    {
+      "key": {
+        "uuid": "3a961159_50617e37",
+        "filename": "shade/__init__.py",
+        "patchSetId": 2
+      },
+      "lineNbr": 727,
+      "author": {
+        "id": 1
+      },
+      "writtenOn": "2015-01-08T22:53:38Z",
+      "side": 1,
+      "message": "Normally I really dislike this pattern because it masks the true cause of errors.  It looks like this specific case probably has the right information assuming there is an http response.  But what if there\u0027s a DNS resolution problem or something?  is that a ClientException or something else?  (If something else, that\u0027s okay, if it\u0027s a ClientException, then we\u0027ve just masked the problem).\n\nSomething we might want to do in the future is to attach the original exception to the new one.  I think python3 makes it easy to wrap, but I think there are still ways to do that in python2.",
+      "revId": "ea922e2cc799527f56ad94bff99401eb5ea2e25d",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": false
+    }
+  ]
+}