On Thu, May 08, 2014 at 10:10:34AM -0400, Abhishek Koneru wrote:
Please review the attached patch with fixes for Ade's comments on IRC.
- Fix all the errors/warnings and some typos shown by PyCharm. - Add Link class which stores information of the Link object. - Add an internal method _submit_revoke_request and for the common code in revoke_cert and revoke_ca_cert. - Mention the need of agent authentication for the methods where it is required. - Default revocation_reason should be REASON_UNSPECIFIED - Add the decorator pki.handle_exceptions() to handle the PKIExceptions sent by the server.
--Abhishek On Mon, 2014-05-05 at 06:46 -0400, Abhishek Koneru wrote:
Please review the attached patch which implements the Cert resource methods in CertClient on the python side.
--Abhishek _______________________________________________ Pki-devel mailing list Pki-devel@redhat.com https://www.redhat.com/mailman/listinfo/pki-devel
This change (along with patch 93) works fine for the examples in the ``main`` method. I'm going to test it some more, though. Some stylistic comments inline below. Cheers, Fraser 8< SNIP >8
class CertSearchRequest(object): + """ + An object of this class is used to store the search parameters + and send them to server. + """
- def __init__(self): - self.serialNumberRangeInUse = False - self.serialTo = None - self.serialFrom = None - self.subjectInUse = False - self.eMail = None - self.commonName = None - self.userID = None - self.orgUnit = None - self.org = None - self.locality = None - self.state = None - self.country = None - self.matchExactly = None - self.status = None - self.revokedBy = None - self.revokedOnFrom = None - self.revokedOnTo = None - self.revocationReason = None - self.issuedBy = None - self.issuedOnFrom = None - self.issuedOnTo = None - self.validNotBeforeFrom = None - self.validNotBeforeTo = None - self.validNotAfterFrom = None - self.validNotAfterTo = None - self.validityOperation = None - self.validityCount = None - self.validityUnit = None - self.certTypeSubEmailCA = None - self.certTypeSubSSLCA = None - self.certTypeSecureEmail = None - self.certTypeSSLClient = None - self.certTypeSSLServer = None - self.revokedByInUse = False - self.revokedOnInUse = False - self.revocationReasonInUse = None - self.issuedByInUse = False - self.issuedOnInUse = False - self.validNotBeforeInUse = False - self.validNotAfterInUse = False - self.validityLengthInUse = False - self.certTypeInUse = False + search_params = {'serial_to': 'serialTo', 'serial_from': 'serialFrom', + 'email': 'eMail', 'common_name': 'commonName', 'user_id': 'userID', + 'org_unit': 'orgUnit', 'org': 'org', 'locality': 'locality', + 'state': 'state', 'country': 'country', 'match_exactly': 'matchExactly', + 'status': 'status', 'revoked_by': 'revokedBy', 'revoked_on_from': 'revokedOnFrom', + 'revoked_on_to': 'revokedOnTo', 'revocation_reason': 'revocationReason', + 'issued_by': 'issuedBy', 'issued_on_from': 'issuedOnFrom', 'issued_on_to': 'issuedOnTo', + 'valid_not_before_from': 'validNotBeforeFrom', 'valid_not_before_to': 'validNotBeforeTo', + 'valid_not_after_from': 'validNotAfterFrom', 'valid_not_after_to': 'validNotAfterTo', + 'validity_operation': 'validityOperation', 'validity_count': 'validityCount', + 'validity_unit': 'validityUnit', 'cert_type_sub_email_ca': 'certTypeSubEmailCA', + 'cert_type_sub_ssl_ca': 'certTypeSubSSLCA', 'cert_type_secure_email': 'certTypeSecureEmail', + 'cert_type_ssl_client': 'certTypeSSLClient', 'cert_type_ssl_server': 'certTypeSSLServer'} + + def __init__(self, **cert_search_params): + """ Constructor """ + + if len(cert_search_params) == 0: + setattr(self, 'serialNumberRangeInUse', True) + + for param in cert_search_params:
Here you could iterate the (key, value) pairs to save keystrokes and a bit of CPU doing ``cert_search_params[param]`` in all those places below. e.g.: for param, value in cert_search_params.viewitems(): ... ``dict.viewitems()`` returns a *dictview* iterator-like object that avoids creating an intermediate list as ``dict.items()`` would.
+ if not param in CertSearchRequest.search_params: + raise ValueError('Invalid search parameter: ' + param) + + if param == 'serial_to' or param == 'serial_from': + setattr(self, CertSearchRequest.search_params[param], cert_search_params[param]) + setattr(self, 'serialNumberRangeInUse', True) + + if param == 'email' or param == 'common_name' or param == 'user_id' or param == 'org_unit' \ + or param == 'org' or param == 'locality' or param == 'state' or param == 'country' \ + or param == 'match_exactly': + setattr(self, CertSearchRequest.search_params[param], cert_search_params[param]) + setattr(self, 'subjectInUse', True)
Here and in a few other places below it might improve readability to test for set membership, e.g.: if param in { 'email', 'common_name', 'user'_id', 'org_unit', 'org', ... }: setattr...
+ + if param == 'status': + setattr(self, CertSearchRequest.search_params[param], cert_search_params[param]) + + if param == 'revoked_by': + setattr(self, CertSearchRequest.search_params[param], cert_search_params[param]) + setattr(self, 'revokedByInUse', True) + + if param == 'revoked_on_from' or param == 'revoked_on_to': + setattr(self, CertSearchRequest.search_params[param], cert_search_params[param]) + setattr(self, 'revokedOnInUse', True) + + if param == 'revocation_reason': + setattr(self, CertSearchRequest.search_params[param], cert_search_params[param]) + setattr(self, 'revocationReasonInUse', True) + + if param == 'issued_by': + setattr(self, CertSearchRequest.search_params[param], cert_search_params[param]) + setattr(self, 'issuedByInUse', True) + + if param == 'issued_on_from' or param == 'issued_on_to': + setattr(self, CertSearchRequest.search_params[param], cert_search_params[param]) + setattr(self, 'issuedOnInUse', True) + + if param == 'valid_not_before_from' or param == 'valid_not_before_to': + setattr(self, CertSearchRequest.search_params[param], cert_search_params[param]) + setattr(self, 'validNotBeforeInUse', True) + + if param == 'valid_not_after_from' or param == 'valid_not_after_to': + setattr(self, CertSearchRequest.search_params[param], cert_search_params[param]) + setattr(self, 'validNotAfterInUse', True) + + if param == 'validity_operation' or param == 'validity_count' or param == 'validity_unit': + setattr(self, CertSearchRequest.search_params[param], cert_search_params[param]) + setattr(self, 'validityLengthInUse', True) + + if param == 'cert_type_sub_email_ca' or param == 'cert_type_sub_ssl_ca' \ + or param == 'cert_type_secure_email' or param == 'cert_type_ssl_client' \ + or param == 'cert_type_ssl_server': + setattr(self, CertSearchRequest.search_params[param], cert_search_params[param]) + setattr(self, 'certTypeInUse', True)
8< SNIP >8