So i addressed all the new comments from Fraser and Ade. Ade is OK with the current set of patches checked in (93, 94, 95-2). I will be submitting new patches which add the remaining methods in ProfileClient and provide a better solution for the property/setter usage in classes like ProfileData, CertEnrollmentRequest and CertReviewResponse. Fraser - if the current code is OK with you please let me know. i can check in the code right away. As per my previous mail - these are addressed:
1) there is an unused class ``ProfilePolicySet``. Is the purpose of this class the same as ``PolicySet``? If so, they should be merged.
The ProfilePolicySet class is being used in the CertReviewResponse class. It is different from the PolicySet class.
The former is a list of ProfilePolicy objects while the latter is a dictionary of {Policy_name, Collection of <Profile_Policy>}.
Both classes are replications of similar definitions on the Java side.
2) as currently implemented, ``PolicySet.from_json`` has no explicit return, therefore returns None
3) ``PolicySet.from_json`` likewise lacks a return statement.
Will be corrected.
One more thing:
4) due to use of ``pki.handle_exceptions`` decorator, docstrings on decorated methods are wrong. You can use ``functools.wraps`` to propagate docstrings through a decorator. See http://stackoverflow.com/questions/1782843/python-decorator-problem-with-doc...
Good catch and great suggestion. I will work on this. Though the link you provided is pretty old, i still do not see any support for showing the arguments of the wrapped function instead of the wrapper function's arguments - when calling help. In any case, i used functools.wraps to atleast show the docstring.
Along with the following comments by Ade. 1. You need to not track workspace.xml - and most likely add it to .gitignore. See http://manski.net/2012/05/which-project-files-under-version-control/#intelli... That said, it appears that Barbican for example, does not in fact check in their .idea files. You might want to ask them about that. I removed the PyCharm project files. (Even barbican doesn't check them in and they do not have any other way of checking the PEP8 standards using the PyCharm files) 2. In CertRevoke, you do check to see if the reason provided is a valid reason, but do this by defining the valid reasons twice - once as a standalone field, and once in an anonymous list. Better to define all the reasons in a named list, and check that list. -- Included in the patch. -- Abhishek