Re: [Pki-devel] [PATCH] 58-2 Minor fixes for patch 58

On 5/31/2013 2:20 PM, Abhishek Koneru wrote: >> 1. In drmclient.py, in http_request() and https_request() suppose the >> NSSConnection supports Context Manager we could use 'with' statement >> too. Could you check? >> >> http://docs.python.org/2/reference/compound_stmts.html#with >> >> 2. Also in http_request() and https_request(), I'm not sure if we really >> need to wrap the original exception, but that's a separate issue. If we >> can remove it we can use 'with' here, or at least nest it. > > Both NSSConnection and httplib.HTTPConnection do not support Context > Manager. Hence, no changes made here. OK. >> 3. In kra.__init__() the self.password = '' assignment can be moved >> before open() so in case of error it will be blank already. This way we >> can use the 'with' statement. > -- Nested the with in the try-except block There's a trailing space on line 419. Please fix before push. >> 4. In pkimanifest.py, in file.write() and read() the error logging >> should really be done by the caller. But here at least we can use 'with' >> nested inside the 'try-except'. > used with for file operations >> > Please review the attached patch. ACK.