On Fri, 2013-05-24 at 12:56 -0500, Endi Sukma Dewata wrote:
On 5/21/2013 12:02 PM, Abhishek Koneru wrote:
> Please review he attached patch with a minor modification in code - to
> use the with construct for handling files.
Some of the 'try-except-finally' clauses probably can be simplified
because sometimes the 'except' only wraps the original exception, or
logs an error, or does simple error handling.
It's also possible to nest 'with' inside 'try-except' (or the other
way
around); one to handle the resource and the other to handle the error.
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.
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
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.
--Abhishek