)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"ae11b3721cbb65c83c4f615000b4b1618e5aafd1","unresolved":false,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2014-07-15 17:06:58 -0500"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add state machine and unit tests for symantec plugin"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Change-Id: Ieb207f91c8251d8ec6601fa113f2501869382e74"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"baada198_71398c3e","line":8,"updated":"2014-07-17 21:22:42.000000000","message":"Please add:\n\nImplements: blueprint add-ssl-ca-support\n\nAlso, change topic branch to bp/add-ssl-ca-support","commit_id":"12a7d520499002a1d08afeaf554eecb144d56213"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"52060456f4754a605c84f1f1fe31ea4b8e607ed9","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Add Certificate Interface \u0026 Symantec Plugin"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Adding the new Certificate interface and Symantec plugin."},{"line_number":10,"context_line":"This includes unit tests as well as a part of the tasking."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: Ieb207f91c8251d8ec6601fa113f2501869382e74"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"baada198_0fbc206b","line":10,"updated":"2014-07-29 03:58:38.000000000","message":"Please add:\n\nImplements: blueprint add-ssl-ca-support\n\nand change topic branch to bp/add-ssl-ca-support","commit_id":"8f503843ebb931f0ff5eff32d4d6a9d967a15968"}],"barbican/plugin/interface/certificate_manager.py":[{"author":{"_account_id":9914,"name":"Ade Lee","email":"alee@redhat.com","username":"alee"},"change_message_id":"fd8eabd8605462ae5d54194c2dba8532ab7138d2","unresolved":false,"context_lines":[{"line_number":146,"context_line":"    CLIENT_DATA_ISSUE_SEEN \u003d \"client data issue seen\""},{"line_number":147,"context_line":"    CA_UNAVAILABLE_FOR_REQUEST \u003d \"CA unavailable for request\""},{"line_number":148,"context_line":"    REQUEST_CANCELED \u003d \"request canceled\""},{"line_number":149,"context_line":"    CONFLICT_WITH_CA \u003d \"conflict with CA\""},{"line_number":150,"context_line":""},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"class ResultDTO(object):"}],"source_content_type":"text/x-python","patch_set":7,"id":"baada198_bfe1edd0","line":149,"updated":"2014-08-02 02:29:44.000000000","message":"Perhaps CONFLICT_WITH_CA is not the best description of this state.  The case I was thinking of here would be for example, trying to cancel or modify a certificate request after the cert has already been issued.\n\nMaybe INVALID_OPERATION or INVALID_STATE_TRANSITION ?","commit_id":"3dc1267de565e467d8dbb35b7c2ec21d3b4db2e3"},{"author":{"_account_id":9914,"name":"Ade Lee","email":"alee@redhat.com","username":"alee"},"change_message_id":"fd8eabd8605462ae5d54194c2dba8532ab7138d2","unresolved":false,"context_lines":[{"line_number":165,"context_line":"        self.status \u003d status"},{"line_number":166,"context_line":"        self.status_message \u003d status_message"},{"line_number":167,"context_line":"        self.certificate \u003d certificate"},{"line_number":168,"context_line":"        self.intermediates \u003d intermediates"},{"line_number":169,"context_line":"        self.retry_msec \u003d retry_msec"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"baada198_ff3c4532","line":168,"updated":"2014-08-02 02:29:44.000000000","message":"I have a question about this field.  Is the expectation that the plugin fill in this field each time issue_certificate_request() is called?\n\nWhat is the format of this field?  PKCS#7? PEM?\nAlso what is the format of the certificate (base64)?","commit_id":"3dc1267de565e467d8dbb35b7c2ec21d3b4db2e3"},{"author":{"_account_id":11970,"name":"Chelsea Winfree","email":"chelsea.winfree@gmail.com","username":"chellygel"},"change_message_id":"a7790197a9220ab20c944000291f8ed874729623","unresolved":false,"context_lines":[{"line_number":165,"context_line":"        self.status \u003d status"},{"line_number":166,"context_line":"        self.status_message \u003d status_message"},{"line_number":167,"context_line":"        self.certificate \u003d certificate"},{"line_number":168,"context_line":"        self.intermediates \u003d intermediates"},{"line_number":169,"context_line":"        self.retry_msec \u003d retry_msec"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"baada198_f3d60b52","line":168,"in_reply_to":"baada198_ff3c4532","updated":"2014-08-04 18:11:34.000000000","message":"To answer your last question, Symantec returns the certificate in base64, but also supports PKCS7.\n\nThe idea behind the intermediates was to store the certificate chain from the intermediate CA. There definitely may be more fields to be returned in the Result DTO, but for the initial CR this seemed logical to keep it low.\n\nIm not sure about the format of the field, i\u0027d have to defer to woodster on that...\n\nLet me know if i answered everything you were looking for.","commit_id":"3dc1267de565e467d8dbb35b7c2ec21d3b4db2e3"},{"author":{"_account_id":9914,"name":"Ade Lee","email":"alee@redhat.com","username":"alee"},"change_message_id":"fd8eabd8605462ae5d54194c2dba8532ab7138d2","unresolved":false,"context_lines":[{"line_number":167,"context_line":"        self.certificate \u003d certificate"},{"line_number":168,"context_line":"        self.intermediates \u003d intermediates"},{"line_number":169,"context_line":"        self.retry_msec \u003d retry_msec"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"class CertificatePluginManager(named.NamedExtensionManager):"},{"line_number":173,"context_line":"    def __init__(self, conf\u003dCONF, invoke_on_load\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":7,"id":"baada198_bfcbad1b","line":170,"updated":"2014-08-02 02:29:44.000000000","message":"What is the expected behavior if retry_msec is set?  That is - after retry_msec, what function gets called?  Or is the expectation that this is just the same function (being retried)?\n\nWe discussed possibly including a mechanism for plugins to specify a function and arguments for the barbican-core to execute after a specified time (maybe defined by retry_msec).  This would give plugin the flexibility to satisfy custom flows, while allowing most plugins to follow the regular flow.  Would those fields be added here?\n(maybe as next_msec, next_function, next_args ?)\n \nAn example of such a flow would be something like this.  In order to issue a certificate, an entry needs to be first added to an authentication database.","commit_id":"3dc1267de565e467d8dbb35b7c2ec21d3b4db2e3"},{"author":{"_account_id":9914,"name":"Ade Lee","email":"alee@redhat.com","username":"alee"},"change_message_id":"d8be0819d07f951d87ec352b231792dbbd0cdc3e","unresolved":false,"context_lines":[{"line_number":167,"context_line":"        self.certificate \u003d certificate"},{"line_number":168,"context_line":"        self.intermediates \u003d intermediates"},{"line_number":169,"context_line":"        self.retry_msec \u003d retry_msec"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"class CertificatePluginManager(named.NamedExtensionManager):"},{"line_number":173,"context_line":"    def __init__(self, conf\u003dCONF, invoke_on_load\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":7,"id":"baada198_e5ca8f9f","line":170,"in_reply_to":"baada198_0a35d2db","updated":"2014-08-05 21:18:27.000000000","message":"The idea behind the extra fields say: next_msec, next_function, next_args is to allow flexibility for plugins that need to manage a custom workflow.  The idea would be that next_function would be any function that is defined within the plugin itself.\n\nMost plugins would not need this mechanism at all.  If a large number of plugins needed a particular function, then we\u0027d consider adding it to the contract.\n\nThis is different from the original proposal. In the original proposal, plugins were given too much responsibility for managing common flows.  Now, we have a framework that allows barbican-core to control most flows - but that also provides the flexibility for plugins that need it.\n\nDogtag has some similar mechanisms.  We provide classes and interfaces to handle most functionality, but we provide a mechanism for folks to write their own plugins -- which people have done.\n\nA note about next_args -- we can probably not provide this field and just have barbican-core pass in the standard args (metadata, etc.).  The plugin can use the metadata to keep track of whatever it needs.","commit_id":"3dc1267de565e467d8dbb35b7c2ec21d3b4db2e3"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"9b72e4c8c2eb85c9361c0f141bcf1e4a4e17f431","unresolved":false,"context_lines":[{"line_number":167,"context_line":"        self.certificate \u003d certificate"},{"line_number":168,"context_line":"        self.intermediates \u003d intermediates"},{"line_number":169,"context_line":"        self.retry_msec \u003d retry_msec"},{"line_number":170,"context_line":""},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"class CertificatePluginManager(named.NamedExtensionManager):"},{"line_number":173,"context_line":"    def __init__(self, conf\u003dCONF, invoke_on_load\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":7,"id":"baada198_0a35d2db","line":170,"in_reply_to":"baada198_bfcbad1b","updated":"2014-08-05 20:36:49.000000000","message":"Ade, retry msec property was indeed intended to retry this method at a later time. We can add another property that optionally specifies a method to be called, but I think that should only be one of the methods defined in CertificatePluginBase so we are always invoking methods in the official plugin interface/contract. In this case, the input parameters don\u0027t have to be specified as they are always the same for each method.","commit_id":"3dc1267de565e467d8dbb35b7c2ec21d3b4db2e3"},{"author":{"_account_id":9914,"name":"Ade Lee","email":"alee@redhat.com","username":"alee"},"change_message_id":"fd8eabd8605462ae5d54194c2dba8532ab7138d2","unresolved":false,"context_lines":[{"line_number":184,"context_line":"        \"\"\"Gets a certificate plugin."},{"line_number":185,"context_line":""},{"line_number":186,"context_line":"        :returns: CertficiatePluginBase plugin implementation"},{"line_number":187,"context_line":"        \"\"\""},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"        if len(self.extensions) \u003c 1:"},{"line_number":190,"context_line":"            raise CertificatePluginNotFound()"}],"source_content_type":"text/x-python","patch_set":7,"id":"baada198_3fdf9dd8","line":187,"updated":"2014-08-02 02:29:44.000000000","message":"It is still useful to have an option plugin_name\u003dNone to allow one to specify a specific plugin.","commit_id":"3dc1267de565e467d8dbb35b7c2ec21d3b4db2e3"},{"author":{"_account_id":10873,"name":"Juan Antonio Osorio Robles","email":"jaosorior@redhat.com","username":"ejuaoso"},"change_message_id":"61102ca1adb1c8577d7be7af12347943a67963db","unresolved":false,"context_lines":[{"line_number":235,"context_line":"        for ext in self.extensions:"},{"line_number":236,"context_line":"            if ext.obj.supports(certificate_spec):"},{"line_number":237,"context_line":"                certificate_plugin \u003d ext.obj"},{"line_number":238,"context_line":"                break"},{"line_number":239,"context_line":"        else:"},{"line_number":240,"context_line":"            raise CertificateSupportedPluginNotFound()"},{"line_number":241,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_5eb93e15","line":238,"updated":"2014-08-06 07:25:47.000000000","message":"Why not return the certificate plugin here, instead of breaking?","commit_id":"0dfc4e6dc06dd3e5b9e16706d365df9e5dccf59f"},{"author":{"_account_id":11970,"name":"Chelsea Winfree","email":"chelsea.winfree@gmail.com","username":"chellygel"},"change_message_id":"2d17298b62eb7c164a802b51382e26d6cb60ef4c","unresolved":false,"context_lines":[{"line_number":235,"context_line":"        for ext in self.extensions:"},{"line_number":236,"context_line":"            if ext.obj.supports(certificate_spec):"},{"line_number":237,"context_line":"                certificate_plugin \u003d ext.obj"},{"line_number":238,"context_line":"                break"},{"line_number":239,"context_line":"        else:"},{"line_number":240,"context_line":"            raise CertificateSupportedPluginNotFound()"},{"line_number":241,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_86bc5d09","line":238,"in_reply_to":"baada198_5eb93e15","updated":"2014-08-06 15:21:05.000000000","message":"I was following the pattern from secret_store and dogtag for these get methods. I think if I were to change it it would be across the board and a separate CR... no?","commit_id":"0dfc4e6dc06dd3e5b9e16706d365df9e5dccf59f"},{"author":{"_account_id":9914,"name":"Ade Lee","email":"alee@redhat.com","username":"alee"},"change_message_id":"0cfe4c934bf349ed7ddb4175d858167032d9553a","unresolved":false,"context_lines":[{"line_number":182,"context_line":"        :param intermediates: Intermediates to be stored in container"},{"line_number":183,"context_line":"        :param retry_msec: Number of milliseconds to wait for retry"},{"line_number":184,"context_line":"        :param retry_method: Method to be called for retry, if None then retry"},{"line_number":185,"context_line":"                             the cuurrent method"},{"line_number":186,"context_line":"        \"\"\""},{"line_number":187,"context_line":"        self.status \u003d status"},{"line_number":188,"context_line":"        self.status_message \u003d status_message"}],"source_content_type":"text/x-python","patch_set":10,"id":"baada198_c6e64ee7","line":185,"range":{"start_line":185,"start_character":33,"end_line":185,"end_character":41},"updated":"2014-08-06 18:35:29.000000000","message":"typo - cuurrent","commit_id":"a01aa59efa45da781e99a7fc4aaa8d369398be27"},{"author":{"_account_id":9914,"name":"Ade Lee","email":"alee@redhat.com","username":"alee"},"change_message_id":"0cfe4c934bf349ed7ddb4175d858167032d9553a","unresolved":false,"context_lines":[{"line_number":220,"context_line":"            raise CertificateSupportedPluginNotFound()"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"        return certificate_plugin"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"    def get_plugin(self, certificate_spec):"},{"line_number":225,"context_line":"        \"\"\"Gets a supporting certificate plugin."},{"line_number":226,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"baada198_86bb7673","line":223,"updated":"2014-08-06 18:35:29.000000000","message":"get_plugin_by_name:  I was not thinking about an extra method.  Rather, add an extra parameter below:\nplugin_name\u003dNone","commit_id":"a01aa59efa45da781e99a7fc4aaa8d369398be27"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"0f397222d006be686f396fb1323c94c2e4b5a20f","unresolved":false,"context_lines":[{"line_number":220,"context_line":"            raise CertificateSupportedPluginNotFound()"},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"        return certificate_plugin"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"    def get_plugin(self, certificate_spec):"},{"line_number":225,"context_line":"        \"\"\"Gets a supporting certificate plugin."},{"line_number":226,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"baada198_892aa9e4","line":223,"in_reply_to":"baada198_86bb7673","updated":"2014-08-06 18:48:08.000000000","message":"Please see comments below.","commit_id":"a01aa59efa45da781e99a7fc4aaa8d369398be27"},{"author":{"_account_id":9914,"name":"Ade Lee","email":"alee@redhat.com","username":"alee"},"change_message_id":"0cfe4c934bf349ed7ddb4175d858167032d9553a","unresolved":false,"context_lines":[{"line_number":229,"context_line":"        :returns: CertficiatePluginBase plugin implementation"},{"line_number":230,"context_line":"        \"\"\""},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"        if len(self.extensions) \u003c 1:"},{"line_number":233,"context_line":"            raise CertificatePluginNotFound()"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":"        for ext in self.extensions:"}],"source_content_type":"text/x-python","patch_set":10,"id":"baada198_e9aa45d2","line":232,"updated":"2014-08-06 18:35:29.000000000","message":"Actually , do we need this check ..","commit_id":"a01aa59efa45da781e99a7fc4aaa8d369398be27"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"0f397222d006be686f396fb1323c94c2e4b5a20f","unresolved":false,"context_lines":[{"line_number":229,"context_line":"        :returns: CertficiatePluginBase plugin implementation"},{"line_number":230,"context_line":"        \"\"\""},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"        if len(self.extensions) \u003c 1:"},{"line_number":233,"context_line":"            raise CertificatePluginNotFound()"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":"        for ext in self.extensions:"}],"source_content_type":"text/x-python","patch_set":10,"id":"baada198_69cf158c","line":232,"in_reply_to":"baada198_e9aa45d2","updated":"2014-08-06 18:48:08.000000000","message":"Good point, #233 would handle the no-extensions-configured condition.","commit_id":"a01aa59efa45da781e99a7fc4aaa8d369398be27"},{"author":{"_account_id":9914,"name":"Ade Lee","email":"alee@redhat.com","username":"alee"},"change_message_id":"0cfe4c934bf349ed7ddb4175d858167032d9553a","unresolved":false,"context_lines":[{"line_number":231,"context_line":""},{"line_number":232,"context_line":"        if len(self.extensions) \u003c 1:"},{"line_number":233,"context_line":"            raise CertificatePluginNotFound()"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":"        for ext in self.extensions:"},{"line_number":236,"context_line":"            if ext.obj.supports(certificate_spec):"},{"line_number":237,"context_line":"                certificate_plugin \u003d ext.obj"}],"source_content_type":"text/x-python","patch_set":10,"id":"baada198_a6407a60","line":234,"updated":"2014-08-06 18:35:29.000000000","message":"then add this line here:\n\nfor ext in self.extensions:\n    if utils.generate_fullname_for(ext.obj) \u003d\u003d plugin_name:\n        return ext.obj","commit_id":"a01aa59efa45da781e99a7fc4aaa8d369398be27"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"0f397222d006be686f396fb1323c94c2e4b5a20f","unresolved":false,"context_lines":[{"line_number":231,"context_line":""},{"line_number":232,"context_line":"        if len(self.extensions) \u003c 1:"},{"line_number":233,"context_line":"            raise CertificatePluginNotFound()"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":"        for ext in self.extensions:"},{"line_number":236,"context_line":"            if ext.obj.supports(certificate_spec):"},{"line_number":237,"context_line":"                certificate_plugin \u003d ext.obj"}],"source_content_type":"text/x-python","patch_set":10,"id":"baada198_c9af417b","line":234,"in_reply_to":"baada198_a6407a60","updated":"2014-08-06 18:48:08.000000000","message":"There are two get flows possible here...one that returns the first plugin that says they support the input data (below) and an alternate flow where you specific the plugin by name. I\u0027m thinking these are distinct use cases that should be handled via two different get/finders. Otherwise you would need something like this:\n\nif plugin_name:\n    for ext in self.extensions:\n        if utils.generate_fullname_for(ext.obj) \u003d\u003d plugin_name:\n            return ext.obj\nelse:\n    for ext in self.extensions:\n        if ext.obj.supports(certificate_spec):\n             return ext.obj\n\nraise CertificateSupportedPluginNotFound()\n\nThis all said, do you foresee the need for a get-plug-by-name use case?","commit_id":"a01aa59efa45da781e99a7fc4aaa8d369398be27"},{"author":{"_account_id":9914,"name":"Ade Lee","email":"alee@redhat.com","username":"alee"},"change_message_id":"0cfe4c934bf349ed7ddb4175d858167032d9553a","unresolved":false,"context_lines":[{"line_number":235,"context_line":"        for ext in self.extensions:"},{"line_number":236,"context_line":"            if ext.obj.supports(certificate_spec):"},{"line_number":237,"context_line":"                certificate_plugin \u003d ext.obj"},{"line_number":238,"context_line":"                break"},{"line_number":239,"context_line":"        else:"},{"line_number":240,"context_line":"            raise CertificateSupportedPluginNotFound()"},{"line_number":241,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"baada198_061cc67a","line":238,"updated":"2014-08-06 18:35:29.000000000","message":"just return ext.obj here.","commit_id":"a01aa59efa45da781e99a7fc4aaa8d369398be27"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"0f397222d006be686f396fb1323c94c2e4b5a20f","unresolved":false,"context_lines":[{"line_number":235,"context_line":"        for ext in self.extensions:"},{"line_number":236,"context_line":"            if ext.obj.supports(certificate_spec):"},{"line_number":237,"context_line":"                certificate_plugin \u003d ext.obj"},{"line_number":238,"context_line":"                break"},{"line_number":239,"context_line":"        else:"},{"line_number":240,"context_line":"            raise CertificateSupportedPluginNotFound()"},{"line_number":241,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"baada198_69d9d527","line":238,"in_reply_to":"baada198_061cc67a","updated":"2014-08-06 18:48:08.000000000","message":"That sounds good. And then just to a raise cert not found as the last line.","commit_id":"a01aa59efa45da781e99a7fc4aaa8d369398be27"},{"author":{"_account_id":9914,"name":"Ade Lee","email":"alee@redhat.com","username":"alee"},"change_message_id":"0cfe4c934bf349ed7ddb4175d858167032d9553a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"baada198_e96325dc","line":243,"updated":"2014-08-06 18:35:29.000000000","message":"All in all, I think the method should look like this:\n\ndef get_plugin(self, certificate_spec, plugin_name\u003d None):\n    \"\"\"Gets a supporting certificate plugin.\n       etc ..\n    \"\"\"\n\n    for ext in self.extensions:\n        if utils.generate_fullname_for(ext.obj) \u003d\u003d plugin_name:\n        return ext.obj\n\n    for ext in self.extensions:\n        if ext.obj.supports(certificate_spec):\n            return ext.obj\n\n    raise CertificatePluginNotFound()","commit_id":"a01aa59efa45da781e99a7fc4aaa8d369398be27"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"0f397222d006be686f396fb1323c94c2e4b5a20f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":10,"id":"baada198_49e971aa","line":243,"in_reply_to":"baada198_e96325dc","updated":"2014-08-06 18:48:08.000000000","message":"If plugin_name is None though, you\u0027ll have a extra loop in there that doesn\u0027t do anything. Please see comments above asking why we need a get by plugin name feature.","commit_id":"a01aa59efa45da781e99a7fc4aaa8d369398be27"},{"author":{"_account_id":9914,"name":"Ade Lee","email":"alee@redhat.com","username":"alee"},"change_message_id":"9632ce91974bfb28a8e229a22712012f231fc00d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"baada198_915c8382","line":210,"updated":"2014-08-07 01:40:37.000000000","message":"This is not correct.  It should be:\n\nfor ext in self.extensions:\n    if ext.obj.supports(certificate_spec):\n        return ext.obj\n\nraise CertificatePluginNotFound()\n\nThat way the exception is thrown only if no exceptions support the cert_spec","commit_id":"7a0030a02acc11201104db334b3a7d06c466c864"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"45c62122484f2d7ea02d4621536ffe7bdbdb7a11","unresolved":false,"context_lines":[{"line_number":21,"context_line":"\"\"\""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"import abc"},{"line_number":24,"context_line":"import six"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"from oslo.config import cfg"},{"line_number":27,"context_line":"from stevedore import named"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_f8d67ff1","line":24,"updated":"2014-08-07 05:35:25.000000000","message":"Nit: six belongs in the third-party section of the imports.","commit_id":"144f1c0bf1adbbea6c056a46968fd68637ced0f7"},{"author":{"_account_id":11970,"name":"Chelsea Winfree","email":"chelsea.winfree@gmail.com","username":"chellygel"},"change_message_id":"6928d7d1d84dfdda1eadbc74c9312698d96edf03","unresolved":false,"context_lines":[{"line_number":21,"context_line":"\"\"\""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"import abc"},{"line_number":24,"context_line":"import six"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"from oslo.config import cfg"},{"line_number":27,"context_line":"from stevedore import named"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_593d98bb","line":24,"in_reply_to":"baada198_f8d67ff1","updated":"2014-08-07 19:29:33.000000000","message":"done. edited secret_store.py also to conform to standard","commit_id":"144f1c0bf1adbbea6c056a46968fd68637ced0f7"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"45c62122484f2d7ea02d4621536ffe7bdbdb7a11","unresolved":false,"context_lines":[{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    This class is the base plugin contract for certificates."},{"line_number":74,"context_line":"    \"\"\""},{"line_number":75,"context_line":"    __metaclass__ \u003d abc.ABCMeta"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"    @abc.abstractmethod"},{"line_number":78,"context_line":"    def issue_certificate_request(self, order_id, order_meta, plugin_meta):"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_b8cc0764","line":75,"updated":"2014-08-07 05:35:25.000000000","message":"remove this line, as it is not necessary when using the @six.add_metaclass","commit_id":"144f1c0bf1adbbea6c056a46968fd68637ced0f7"},{"author":{"_account_id":11970,"name":"Chelsea Winfree","email":"chelsea.winfree@gmail.com","username":"chellygel"},"change_message_id":"6928d7d1d84dfdda1eadbc74c9312698d96edf03","unresolved":false,"context_lines":[{"line_number":72,"context_line":""},{"line_number":73,"context_line":"    This class is the base plugin contract for certificates."},{"line_number":74,"context_line":"    \"\"\""},{"line_number":75,"context_line":"    __metaclass__ \u003d abc.ABCMeta"},{"line_number":76,"context_line":""},{"line_number":77,"context_line":"    @abc.abstractmethod"},{"line_number":78,"context_line":"    def issue_certificate_request(self, order_id, order_meta, plugin_meta):"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_795fbc10","line":75,"in_reply_to":"baada198_b8cc0764","updated":"2014-08-07 19:29:33.000000000","message":"done.","commit_id":"144f1c0bf1adbbea6c056a46968fd68637ced0f7"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"45c62122484f2d7ea02d4621536ffe7bdbdb7a11","unresolved":false,"context_lines":[{"line_number":150,"context_line":"        raise NotImplementedError  # pragma: no cover"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"class CertificateStatus(object):"},{"line_number":154,"context_line":"    \"\"\"Defines statuses for certificate request process.\"\"\""},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"    CERTIFICATE_GENERATED \u003d \"certificate generated\""}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_380e3790","line":153,"updated":"2014-08-07 05:35:25.000000000","message":"This still really feels like it should be an enum.  Anyone else get that feeling?","commit_id":"144f1c0bf1adbbea6c056a46968fd68637ced0f7"},{"author":{"_account_id":10873,"name":"Juan Antonio Osorio Robles","email":"jaosorior@redhat.com","username":"ejuaoso"},"change_message_id":"2809d2b49fc86c1fd00bb7b410aa0a7dc59033c0","unresolved":false,"context_lines":[{"line_number":150,"context_line":"        raise NotImplementedError  # pragma: no cover"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"class CertificateStatus(object):"},{"line_number":154,"context_line":"    \"\"\"Defines statuses for certificate request process.\"\"\""},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"    CERTIFICATE_GENERATED \u003d \"certificate generated\""}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_e1b96a17","line":153,"in_reply_to":"baada198_380e3790","updated":"2014-08-07 07:20:16.000000000","message":"Yeah... Actually now that you mention it, I had completely forgotten about the existance of Enum in python :o","commit_id":"144f1c0bf1adbbea6c056a46968fd68637ced0f7"},{"author":{"_account_id":11970,"name":"Chelsea Winfree","email":"chelsea.winfree@gmail.com","username":"chellygel"},"change_message_id":"6928d7d1d84dfdda1eadbc74c9312698d96edf03","unresolved":false,"context_lines":[{"line_number":150,"context_line":"        raise NotImplementedError  # pragma: no cover"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"class CertificateStatus(object):"},{"line_number":154,"context_line":"    \"\"\"Defines statuses for certificate request process.\"\"\""},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"    CERTIFICATE_GENERATED \u003d \"certificate generated\""}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_d970c8a9","line":153,"in_reply_to":"baada198_380e3790","updated":"2014-08-07 19:29:33.000000000","message":"enum is not in global requirements for openstack, so we cant use it here :(","commit_id":"144f1c0bf1adbbea6c056a46968fd68637ced0f7"}],"barbican/plugin/interface/certificates.py":[{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"ae11b3721cbb65c83c4f615000b4b1618e5aafd1","unresolved":false,"context_lines":[{"line_number":20,"context_line":"implementations. Hence do not place vendor-specific content in this module."},{"line_number":21,"context_line":"\"\"\""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"import abc"},{"line_number":24,"context_line":"import six"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"baada198_b342a866","line":23,"updated":"2014-07-17 21:22:42.000000000","message":"Nit: insert blank line between different groups of imports.","commit_id":"12a7d520499002a1d08afeaf554eecb144d56213"},{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"b5bee885f683e7d6fe3f6ed89291ee7c02c7d24e","unresolved":false,"context_lines":[{"line_number":92,"context_line":"        :param tasking: Asynchronous tasking support to allow plugins to"},{"line_number":93,"context_line":"                        invoke follow-on processing by Barbican worker"},{"line_number":94,"context_line":"                        processes."},{"line_number":95,"context_line":"        :returns: None"},{"line_number":96,"context_line":"        \"\"\""},{"line_number":97,"context_line":"        raise NotImplementedError  # pragma: no cover"},{"line_number":98,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"baada198_b2a96926","line":95,"updated":"2014-07-17 19:32:39.000000000","message":"Can an implementer of a plugin return a value, or does it have to be none?","commit_id":"12a7d520499002a1d08afeaf554eecb144d56213"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"2eeaa19d6053a022fc5799f61b0f53f82178d4a2","unresolved":false,"context_lines":[{"line_number":92,"context_line":"        :param tasking: Asynchronous tasking support to allow plugins to"},{"line_number":93,"context_line":"                        invoke follow-on processing by Barbican worker"},{"line_number":94,"context_line":"                        processes."},{"line_number":95,"context_line":"        :returns: None"},{"line_number":96,"context_line":"        \"\"\""},{"line_number":97,"context_line":"        raise NotImplementedError  # pragma: no cover"},{"line_number":98,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"baada198_37b704f7","line":95,"in_reply_to":"baada198_b14324ae","updated":"2014-07-17 22:17:02.000000000","message":"Good point. The approach in this CR is a no-return one. Per recent discussions this might change though...I\u0027ll add a link to etherpad discussion about it.","commit_id":"12a7d520499002a1d08afeaf554eecb144d56213"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"ae11b3721cbb65c83c4f615000b4b1618e5aafd1","unresolved":false,"context_lines":[{"line_number":92,"context_line":"        :param tasking: Asynchronous tasking support to allow plugins to"},{"line_number":93,"context_line":"                        invoke follow-on processing by Barbican worker"},{"line_number":94,"context_line":"                        processes."},{"line_number":95,"context_line":"        :returns: None"},{"line_number":96,"context_line":"        \"\"\""},{"line_number":97,"context_line":"        raise NotImplementedError  # pragma: no cover"},{"line_number":98,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"baada198_b14324ae","line":95,"in_reply_to":"baada198_b2a96926","updated":"2014-07-17 21:22:42.000000000","message":"+1 :returns: None is not very helpful here.  If there is nothing to return, then leave the :returns: part out.","commit_id":"12a7d520499002a1d08afeaf554eecb144d56213"},{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"b5bee885f683e7d6fe3f6ed89291ee7c02c7d24e","unresolved":false,"context_lines":[{"line_number":114,"context_line":"        :param tasking: Asynchronous tasking support to allow plugins to"},{"line_number":115,"context_line":"                        invoke follow-on processing by Barbican worker"},{"line_number":116,"context_line":"                        processes."},{"line_number":117,"context_line":"        :returns: None"},{"line_number":118,"context_line":"        \"\"\""},{"line_number":119,"context_line":"        raise NotImplementedError  # pragma: no cover"},{"line_number":120,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"baada198_52a2cd4b","line":117,"updated":"2014-07-17 19:32:39.000000000","message":"is this true?  Would someone who implements this return none?","commit_id":"12a7d520499002a1d08afeaf554eecb144d56213"},{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"b5bee885f683e7d6fe3f6ed89291ee7c02c7d24e","unresolved":false,"context_lines":[{"line_number":130,"context_line":"    __metaclass__ \u003d abc.ABCMeta"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"    @abc.abstractmethod"},{"line_number":133,"context_line":"    def notify_ca_order_sent(self, order_id):"},{"line_number":134,"context_line":"        \"\"\"Notify that an SSL certificate order has be placed with CA."},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"        :param order_id: Barbican order ID"}],"source_content_type":"text/x-python","patch_set":1,"id":"baada198_b257a925","line":133,"updated":"2014-07-17 19:32:39.000000000","message":"are these event objects, or functions to raise these events?","commit_id":"12a7d520499002a1d08afeaf554eecb144d56213"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"2eeaa19d6053a022fc5799f61b0f53f82178d4a2","unresolved":false,"context_lines":[{"line_number":130,"context_line":"    __metaclass__ \u003d abc.ABCMeta"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"    @abc.abstractmethod"},{"line_number":133,"context_line":"    def notify_ca_order_sent(self, order_id):"},{"line_number":134,"context_line":"        \"\"\"Notify that an SSL certificate order has be placed with CA."},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"        :param order_id: Barbican order ID"}],"source_content_type":"text/x-python","patch_set":1,"id":"baada198_9719b024","line":133,"in_reply_to":"baada198_b257a925","updated":"2014-07-17 22:17:02.000000000","message":"They are methods that could raise events, or else just log that something happened, depending on the implementation provided.","commit_id":"12a7d520499002a1d08afeaf554eecb144d56213"},{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"b5bee885f683e7d6fe3f6ed89291ee7c02c7d24e","unresolved":false,"context_lines":[{"line_number":171,"context_line":"        \"\"\"Store the SSL certificate."},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"        :param ssl_cert: SSL certificate to store."},{"line_number":174,"context_line":"        :returns: None"},{"line_number":175,"context_line":"        \"\"\""},{"line_number":176,"context_line":"        #TODO(jwood) What is the best format to use here?"},{"line_number":177,"context_line":"        #   What about intermediate certs?"}],"source_content_type":"text/x-python","patch_set":1,"id":"baada198_52156d47","line":174,"updated":"2014-07-17 19:32:39.000000000","message":"same as above - is None the only possibility for plugins?","commit_id":"12a7d520499002a1d08afeaf554eecb144d56213"},{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"b5bee885f683e7d6fe3f6ed89291ee7c02c7d24e","unresolved":false,"context_lines":[{"line_number":179,"context_line":"        raise NotImplementedError  # pragma: no cover"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"    def find_order_for(self, plugin_meta_key):"},{"line_number":182,"context_line":"        \"\"\"Lookup via a key an order ID from its plugin meta data.\"\"\""},{"line_number":183,"context_line":"        #TODO(jwood) This would support batch processing behaviors."},{"line_number":184,"context_line":"        raise NotImplementedError  # pragma: no cover"},{"line_number":185,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"baada198_0d02fa08","line":182,"updated":"2014-07-17 19:32:39.000000000","message":"add :param and :returns here to be consistent","commit_id":"12a7d520499002a1d08afeaf554eecb144d56213"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"2eeaa19d6053a022fc5799f61b0f53f82178d4a2","unresolved":false,"context_lines":[{"line_number":179,"context_line":"        raise NotImplementedError  # pragma: no cover"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"    def find_order_for(self, plugin_meta_key):"},{"line_number":182,"context_line":"        \"\"\"Lookup via a key an order ID from its plugin meta data.\"\"\""},{"line_number":183,"context_line":"        #TODO(jwood) This would support batch processing behaviors."},{"line_number":184,"context_line":"        raise NotImplementedError  # pragma: no cover"},{"line_number":185,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"baada198_f7a3ccae","line":182,"in_reply_to":"baada198_0d02fa08","updated":"2014-07-17 22:17:02.000000000","message":"Sounds good.","commit_id":"12a7d520499002a1d08afeaf554eecb144d56213"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"52060456f4754a605c84f1f1fe31ea4b8e607ed9","unresolved":false,"context_lines":[{"line_number":74,"context_line":"                            this plugin. Plugins may also update/add"},{"line_number":75,"context_line":"                            information here which Barbican will persist"},{"line_number":76,"context_line":"                            on their behalf."},{"line_number":77,"context_line":"        :returns: ResultDTO"},{"line_number":78,"context_line":"        \"\"\""},{"line_number":79,"context_line":"        raise NotImplementedError  # pragma: no cover"},{"line_number":80,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"baada198_bb6a9380","line":77,"updated":"2014-07-29 03:58:38.000000000","message":"Consider adding reST links in docstrings, and expanding on what someone implementing this interface needs to return in the DTO:\n\n:returns: A :class:`ResultDTO` instance containing the result populated by the plugin implementation.\n:rtype: :class:`ResultDTO`","commit_id":"8f503843ebb931f0ff5eff32d4d6a9d967a15968"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"52060456f4754a605c84f1f1fe31ea4b8e607ed9","unresolved":false,"context_lines":[{"line_number":121,"context_line":"        raise NotImplementedError  # pragma: no cover"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"class CertificateStatus(object):"},{"line_number":125,"context_line":"    \"\"\"Defines statuses for certificate request process.\"\"\""},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"    CERTIFICATE_GENERATED \u003d \"certificate generated\""}],"source_content_type":"text/x-python","patch_set":2,"id":"baada198_fb124bfc","line":124,"updated":"2014-07-29 03:58:38.000000000","message":"Should we consider using an enum for this class?  Not sure if the pythonistas are embracing the new enums in 3.4","commit_id":"8f503843ebb931f0ff5eff32d4d6a9d967a15968"},{"author":{"_account_id":9914,"name":"Ade Lee","email":"alee@redhat.com","username":"alee"},"change_message_id":"4a630fe418fc97e9e820fab8330687ecf6b0d7fe","unresolved":false,"context_lines":[{"line_number":136,"context_line":"    WAITING_FOR_CA \u003d \"waiting for CA\""},{"line_number":137,"context_line":"    CLIENT_DATA_ISSUE_SEEN \u003d \"client data issue seen\""},{"line_number":138,"context_line":"    CA_UNAVAILABLE_FOR_REQUEST \u003d \"CA unavailable for request\""},{"line_number":139,"context_line":"    REQUEST_CANCELED \u003d \"request canceled\""},{"line_number":140,"context_line":""},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"class ResultDTO(object):"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_d4ac5f8b","line":139,"updated":"2014-07-29 15:51:30.000000000","message":"I think you need a CONFLICT status as well.  For example, what happens if you try to cancel an order for which the cert has already been issued?","commit_id":"ee58fe1f0007b4f332db184db52f4a7ce5ddb05d"},{"author":{"_account_id":9914,"name":"Ade Lee","email":"alee@redhat.com","username":"alee"},"change_message_id":"4a630fe418fc97e9e820fab8330687ecf6b0d7fe","unresolved":false,"context_lines":[{"line_number":143,"context_line":"    \"\"\"This object is the result data transfer object (DTO).\"\"\""},{"line_number":144,"context_line":"    def __init__(self, status, status_message\u003dNone, certificate\u003dNone,"},{"line_number":145,"context_line":"                 private_key\u003dNone, intermediates\u003dNone, retry_msec\u003d3600000):"},{"line_number":146,"context_line":"        \"\"\"Creates a new ResultDTO."},{"line_number":147,"context_line":""},{"line_number":148,"context_line":"        :param status: Status for cert order."},{"line_number":149,"context_line":"        :param status_message: Message to explain status type."}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_945ae7f3","line":146,"updated":"2014-07-29 15:51:30.000000000","message":"An hour is a pretty long retry time.  Dogtag will almost certainly shorten this.  I would suggest adding a parameter to the config for the Symantec plugin for operators to configure the timeout.\n\nShould we add a config parameter for this default?","commit_id":"ee58fe1f0007b4f332db184db52f4a7ce5ddb05d"},{"author":{"_account_id":9914,"name":"Ade Lee","email":"alee@redhat.com","username":"alee"},"change_message_id":"4a630fe418fc97e9e820fab8330687ecf6b0d7fe","unresolved":false,"context_lines":[{"line_number":156,"context_line":"        self.status \u003d status"},{"line_number":157,"context_line":"        self.status_message \u003d status_message"},{"line_number":158,"context_line":"        self.certificate \u003d certificate"},{"line_number":159,"context_line":"        self.private_key \u003d private_key"},{"line_number":160,"context_line":"        self.intermediates \u003d intermediates"},{"line_number":161,"context_line":"        self.retry_msec \u003d retry_msec"},{"line_number":162,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_3f19f9d1","line":159,"updated":"2014-07-29 15:51:30.000000000","message":"Why do we have the private key here?  Are we expecting the plugin to generate a private key?\n\nOther than the fact that we should keep the cert plugin and key gen plugins separate, and that the cert plugin only needs the CSR (which includes the public key), its generally good practice not to expose key material like this outside of a crypto token.","commit_id":"ee58fe1f0007b4f332db184db52f4a7ce5ddb05d"},{"author":{"_account_id":9914,"name":"Ade Lee","email":"alee@redhat.com","username":"alee"},"change_message_id":"4a630fe418fc97e9e820fab8330687ecf6b0d7fe","unresolved":false,"context_lines":[{"line_number":173,"context_line":"        )"},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"    def get_plugin(self):"},{"line_number":176,"context_line":"        \"\"\"Gets a certificate plugin."},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"        :returns: CertficiatePluginBase plugin implementation"},{"line_number":179,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_141c1714","line":176,"updated":"2014-07-29 15:51:30.000000000","message":"We have found that for the storage plugins, it is useful to add an optional parameter plugin_name\u003dNone to get a specific plugin.  I suspect that this will be needed here too.  Might as well add it.","commit_id":"ee58fe1f0007b4f332db184db52f4a7ce5ddb05d"}],"barbican/plugin/symantec.py":[{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"053a93a4c56ec90800a6d945e9fc4c71e5b3d4bf","unresolved":false,"context_lines":[{"line_number":40,"context_line":"            return cert.ResultDTO(cert.CertificateStatus.WAITING_FOR_CA)"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        else:"},{"line_number":43,"context_line":"            return cert.ResultDTO(cert.CertificateStatus."},{"line_number":44,"context_line":"                                  CA_UNAVAILABLE_FOR_REQUEST)"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    def modify_certificate_request(self, order_id, order_meta, plugin_meta):"}],"source_content_type":"text/x-python","patch_set":10,"id":"baada198_12ac0388","line":43,"updated":"2014-08-06 17:03:49.000000000","message":"what happens to the error message and can_retry flag?  Sees they get lost?","commit_id":"a01aa59efa45da781e99a7fc4aaa8d369398be27"},{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"053a93a4c56ec90800a6d945e9fc4c71e5b3d4bf","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        \"\"\""},{"line_number":91,"context_line":"        #TODO(chellygel): Research what certificate types are supported by"},{"line_number":92,"context_line":"        # symantec. Returning True for testing purposes"},{"line_number":93,"context_line":"        return True"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"def _ca_create_order(self, order_meta, plugin_meta):"}],"source_content_type":"text/x-python","patch_set":10,"id":"baada198_b2d35700","line":93,"updated":"2014-08-06 17:03:49.000000000","message":"this will only be valid at a point in time, correct?  Each time the list of types is updated then we become outdated.","commit_id":"a01aa59efa45da781e99a7fc4aaa8d369398be27"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"1329a0334b15865dd84a20be00958be839ee8549","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        \"\"\""},{"line_number":91,"context_line":"        #TODO(chellygel): Research what certificate types are supported by"},{"line_number":92,"context_line":"        # symantec. Returning True for testing purposes"},{"line_number":93,"context_line":"        return True"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"def _ca_create_order(self, order_meta, plugin_meta):"}],"source_content_type":"text/x-python","patch_set":10,"id":"baada198_8f0469c6","line":93,"in_reply_to":"baada198_0303d811","updated":"2014-08-06 19:41:45.000000000","message":"Steve, this method should not be reaching out to external resources, since it will (eventually) be used to support synchronous API-side validation. We might indeed have async failures occur if symantec changes things, but those events will eventually be surfaced/monitored so we can update the valid list. That said, I would be surprised if this lists changes often?","commit_id":"a01aa59efa45da781e99a7fc4aaa8d369398be27"},{"author":{"_account_id":11970,"name":"Chelsea Winfree","email":"chelsea.winfree@gmail.com","username":"chellygel"},"change_message_id":"b74c15f88a6fe772fdc24d0afb4dfb599e2905e2","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        \"\"\""},{"line_number":91,"context_line":"        #TODO(chellygel): Research what certificate types are supported by"},{"line_number":92,"context_line":"        # symantec. Returning True for testing purposes"},{"line_number":93,"context_line":"        return True"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"def _ca_create_order(self, order_meta, plugin_meta):"}],"source_content_type":"text/x-python","patch_set":10,"id":"baada198_0303d811","line":93,"in_reply_to":"baada198_b2d35700","updated":"2014-08-06 19:34:06.000000000","message":"true. i\u0027ll have to check and see if the symantec api offers a list of valid order-able certs so we wont have that issue.","commit_id":"a01aa59efa45da781e99a7fc4aaa8d369398be27"},{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"053a93a4c56ec90800a6d945e9fc4c71e5b3d4bf","unresolved":false,"context_lines":[{"line_number":99,"context_line":"    :returns: tuple with success, error message, and can retry"},{"line_number":100,"context_line":"    \"\"\""},{"line_number":101,"context_line":"    #TODO(jwood) Submit order to CA, get partner order id and"},{"line_number":102,"context_line":"    #   then add to the plugin meta data."},{"line_number":103,"context_line":"    return True, None, None"}],"source_content_type":"text/x-python","patch_set":10,"id":"baada198_924493b9","line":102,"updated":"2014-08-06 17:03:49.000000000","message":"so for now this is just a no-op - right?","commit_id":"a01aa59efa45da781e99a7fc4aaa8d369398be27"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"1329a0334b15865dd84a20be00958be839ee8549","unresolved":false,"context_lines":[{"line_number":99,"context_line":"    :returns: tuple with success, error message, and can retry"},{"line_number":100,"context_line":"    \"\"\""},{"line_number":101,"context_line":"    #TODO(jwood) Submit order to CA, get partner order id and"},{"line_number":102,"context_line":"    #   then add to the plugin meta data."},{"line_number":103,"context_line":"    return True, None, None"}],"source_content_type":"text/x-python","patch_set":10,"id":"baada198_0ff059d9","line":102,"in_reply_to":"baada198_03a75852","updated":"2014-08-06 19:41:45.000000000","message":"There is a dependent CR that fills this in I believe.","commit_id":"a01aa59efa45da781e99a7fc4aaa8d369398be27"},{"author":{"_account_id":11970,"name":"Chelsea Winfree","email":"chelsea.winfree@gmail.com","username":"chellygel"},"change_message_id":"b74c15f88a6fe772fdc24d0afb4dfb599e2905e2","unresolved":false,"context_lines":[{"line_number":99,"context_line":"    :returns: tuple with success, error message, and can retry"},{"line_number":100,"context_line":"    \"\"\""},{"line_number":101,"context_line":"    #TODO(jwood) Submit order to CA, get partner order id and"},{"line_number":102,"context_line":"    #   then add to the plugin meta data."},{"line_number":103,"context_line":"    return True, None, None"}],"source_content_type":"text/x-python","patch_set":10,"id":"baada198_03a75852","line":102,"in_reply_to":"baada198_924493b9","updated":"2014-08-06 19:34:06.000000000","message":"yep, this will have to wait on the orders piece","commit_id":"a01aa59efa45da781e99a7fc4aaa8d369398be27"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"45c62122484f2d7ea02d4621536ffe7bdbdb7a11","unresolved":false,"context_lines":[{"line_number":31,"context_line":"                            this plugin. Plugins may also update/add"},{"line_number":32,"context_line":"                            information here which Barbican will persist"},{"line_number":33,"context_line":"                            on their behalf."},{"line_number":34,"context_line":"        :returns: ResultDTO"},{"line_number":35,"context_line":"        \"\"\""},{"line_number":36,"context_line":"        successful, error_msg, can_retry \u003d _ca_create_order(order_meta,"},{"line_number":37,"context_line":"                                                            plugin_meta)"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_38c39707","line":34,"updated":"2014-08-07 05:35:25.000000000","message":"Consider adding ReST links here to\n\n:class:`barbican.plugin.interface.certificate_manager.ResultDTO`","commit_id":"144f1c0bf1adbbea6c056a46968fd68637ced0f7"}],"barbican/tasks/certificate_resources.py":[{"author":{"_account_id":10873,"name":"Juan Antonio Osorio Robles","email":"jaosorior@redhat.com","username":"ejuaoso"},"change_message_id":"61102ca1adb1c8577d7be7af12347943a67963db","unresolved":false,"context_lines":[{"line_number":63,"context_line":"    \"\"\"Add plugin metadata to an order.\"\"\""},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    if not plugin_meta:"},{"line_number":66,"context_line":"        plugin_meta \u003d dict()"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    repos.order_plugin_meta_repo.save(plugin_meta, order_model)"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_3e6aca82","line":66,"updated":"2014-08-06 07:25:47.000000000","message":"when would plugin_meta be something other than a dict (filled or empty)?","commit_id":"0dfc4e6dc06dd3e5b9e16706d365df9e5dccf59f"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"ba1ed3137ed2276b1a94606f4c4077174c30c323","unresolved":false,"context_lines":[{"line_number":63,"context_line":"    \"\"\"Add plugin metadata to an order.\"\"\""},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    if not plugin_meta:"},{"line_number":66,"context_line":"        plugin_meta \u003d dict()"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    repos.order_plugin_meta_repo.save(plugin_meta, order_model)"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_c3fe4706","line":66,"in_reply_to":"baada198_3e6aca82","updated":"2014-08-06 15:18:03.000000000","message":"Only a dict is valid here, so are you suggesting a check such as this instead?:\n\nif not isinstance(plugin_meta, dict):\n    plugin_meta \u003d dict()","commit_id":"0dfc4e6dc06dd3e5b9e16706d365df9e5dccf59f"}],"barbican/tasks/certificates.py":[{"author":{"_account_id":9914,"name":"Ade Lee","email":"alee@redhat.com","username":"alee"},"change_message_id":"4a630fe418fc97e9e820fab8330687ecf6b0d7fe","unresolved":false,"context_lines":[{"line_number":42,"context_line":"        pass"},{"line_number":43,"context_line":"    else:"},{"line_number":44,"context_line":"        raise NotImplementedError  # pragma: no cover"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    # Save plugin order plugin state"},{"line_number":47,"context_line":"    _save_plugin_metadata(order_model, plugin_meta, repos)"},{"line_number":48,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_d1955133","line":45,"updated":"2014-07-29 15:51:30.000000000","message":"Is this the right error to be raising here?  This is really a problem where the plugin returns some status that is unknown to Barbican Core - which, as the number of plugins proliferates, could happen.\n\nI think it would be more useful to throw a more meaningful and detailed exception here, noting the status that was returned.","commit_id":"ee58fe1f0007b4f332db184db52f4a7ce5ddb05d"},{"author":{"_account_id":9914,"name":"Ade Lee","email":"alee@redhat.com","username":"alee"},"change_message_id":"4a630fe418fc97e9e820fab8330687ecf6b0d7fe","unresolved":false,"context_lines":[{"line_number":49,"context_line":""},{"line_number":50,"context_line":"def _get_plugin_meta(order_model):"},{"line_number":51,"context_line":"    meta_dict \u003d dict((k, v.value) for (k, v) in"},{"line_number":52,"context_line":"                     order_model.order_plugin_meta.items())"},{"line_number":53,"context_line":"    return meta_dict"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_d1e391d6","line":52,"updated":"2014-07-29 15:51:30.000000000","message":"Do we need a check for order_model is None?","commit_id":"ee58fe1f0007b4f332db184db52f4a7ce5ddb05d"}],"barbican/tests/plugin/test_symantec.py":[{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"52060456f4754a605c84f1f1fe31ea4b8e607ed9","unresolved":false,"context_lines":[{"line_number":46,"context_line":"            plugin_meta"},{"line_number":47,"context_line":"        )"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"        assert result.status \u003d\u003d \"waiting for CA\""},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    @mock.patch(\u0027barbican.plugin.symantec._ca_create_order\u0027)"},{"line_number":52,"context_line":"    def test_unsuccessful_initial_order(self, mock_ca_create_order):"}],"source_content_type":"text/x-python","patch_set":2,"id":"baada198_fb97ab43","line":49,"updated":"2014-07-29 03:58:38.000000000","message":"Prefer self.assertEqual(...) instead of the assert statement.","commit_id":"8f503843ebb931f0ff5eff32d4d6a9d967a15968"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"52060456f4754a605c84f1f1fe31ea4b8e607ed9","unresolved":false,"context_lines":[{"line_number":61,"context_line":"            plugin_meta"},{"line_number":62,"context_line":"        )"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"        assert result.status \u003d\u003d \"CA unavailable for request\""},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def test_should_raise_unsupported_initial_order(self):"},{"line_number":67,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"baada198_7b839b03","line":64,"updated":"2014-07-29 03:58:38.000000000","message":"Ditto","commit_id":"8f503843ebb931f0ff5eff32d4d6a9d967a15968"},{"author":{"_account_id":10873,"name":"Juan Antonio Osorio Robles","email":"jaosorior@redhat.com","username":"ejuaoso"},"change_message_id":"dfd045a414005c6fb363942d8bd202139c71286e","unresolved":false,"context_lines":[{"line_number":46,"context_line":"            plugin_meta"},{"line_number":47,"context_line":"        )"},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"        self.assertEquals(result.status, \"waiting for CA\")"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    @mock.patch(\u0027barbican.plugin.symantec._ca_create_order\u0027)"},{"line_number":52,"context_line":"    def test_unsuccessful_initial_order(self, mock_ca_create_order):"}],"source_content_type":"text/x-python","patch_set":3,"id":"baada198_275ca480","line":49,"updated":"2014-07-29 06:48:23.000000000","message":"use assertEqual instead of assertEquals (the former is deprecated)","commit_id":"d23aecf54d549a02e19ab32c2e6d5411f1435cab"},{"author":{"_account_id":8623,"name":"Kaitlin Farr","email":"kaitlin.farr@jhuapl.edu","username":"kaitlin.farr"},"change_message_id":"57553d19c4d0f11a646f53870e3da6716afb4989","unresolved":false,"context_lines":[{"line_number":16,"context_line":"import mock"},{"line_number":17,"context_line":"import testtools"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"import barbican.plugin.symantec as sym"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"class WhenTestingSymantecPlugin(testtools.TestCase):"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_6cfb917b","line":19,"updated":"2014-07-30 21:11:39.000000000","message":"It looks like the WIP patch \u0027First attempt at adding the symantecssl library\u0027 is failing these unit tests because symantecssl isn\u0027t available. Perhaps these tests could be skipped unless the library is available, at least for now?  I\u0027m trying a similar thing for KMIP, and it looks like the unit tests are also being skipped for dogtag: https://github.com/openstack/barbican/blob/167f541022d5d7fb36a4469d6e9095bbb7c7736f/barbican/tests/plugin/test_dogtag.py#L21","commit_id":"ee58fe1f0007b4f332db184db52f4a7ce5ddb05d"},{"author":{"_account_id":11661,"name":"Kevin Bishop","email":"ktb2012@att.net","username":"kbishop711"},"change_message_id":"adc9ace8c652d80516be3547877c2ae4eb823efc","unresolved":false,"context_lines":[{"line_number":16,"context_line":"import mock"},{"line_number":17,"context_line":"import testtools"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"import barbican.plugin.symantec as sym"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"class WhenTestingSymantecPlugin(testtools.TestCase):"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_cb3539f7","line":19,"in_reply_to":"baada198_6cfb917b","updated":"2014-07-31 20:50:29.000000000","message":"+1 Currently symantecssl isn\u0027t in requirements.txt","commit_id":"ee58fe1f0007b4f332db184db52f4a7ce5ddb05d"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"a57b2b35b82d319f06aeab71e0361fe53ab9d533","unresolved":false,"context_lines":[{"line_number":16,"context_line":"import mock"},{"line_number":17,"context_line":"import testtools"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"import barbican.plugin.symantec as sym"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"class WhenTestingSymantecPlugin(testtools.TestCase):"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_0ead7fea","line":19,"in_reply_to":"baada198_cb3539f7","updated":"2014-07-31 20:58:22.000000000","message":"Good point, that should be import protected similar to what Dogtag unit tests do.","commit_id":"ee58fe1f0007b4f332db184db52f4a7ce5ddb05d"},{"author":{"_account_id":9914,"name":"Ade Lee","email":"alee@redhat.com","username":"alee"},"change_message_id":"4a630fe418fc97e9e820fab8330687ecf6b0d7fe","unresolved":false,"context_lines":[{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    @mock.patch(\u0027barbican.plugin.symantec._ca_create_order\u0027)"},{"line_number":52,"context_line":"    def test_unsuccessful_initial_order(self, mock_ca_create_order):"},{"line_number":53,"context_line":"        mock_ca_create_order.return_value \u003d (False, self.error_msg, True)"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"        order_id \u003d \u00271234\u0027"},{"line_number":56,"context_line":"        plugin_meta \u003d dict()"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_344d9bc6","line":53,"updated":"2014-07-29 15:51:30.000000000","message":"Need to change the name of this method\ntest_unsuccessful_issue_certificate_request ?","commit_id":"ee58fe1f0007b4f332db184db52f4a7ce5ddb05d"},{"author":{"_account_id":9914,"name":"Ade Lee","email":"alee@redhat.com","username":"alee"},"change_message_id":"4a630fe418fc97e9e820fab8330687ecf6b0d7fe","unresolved":false,"context_lines":[{"line_number":64,"context_line":"        self.assertEqual(result.status, \"CA unavailable for request\")"},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"    def test_should_raise_unsupported_initial_order(self):"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"        order_id \u003d \u00271234\u0027"},{"line_number":69,"context_line":"        plugin_meta \u003d dict()"},{"line_number":70,"context_line":"        self.assertRaises("}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_54e12f9a","line":67,"updated":"2014-07-29 15:51:30.000000000","message":"The name of this method is wrong too.","commit_id":"ee58fe1f0007b4f332db184db52f4a7ce5ddb05d"},{"author":{"_account_id":10873,"name":"Juan Antonio Osorio Robles","email":"jaosorior@redhat.com","username":"ejuaoso"},"change_message_id":"61102ca1adb1c8577d7be7af12347943a67963db","unresolved":false,"context_lines":[{"line_number":29,"context_line":"    def setUp(self):"},{"line_number":30,"context_line":"        super(WhenTestingSymantecPlugin, self).setUp()"},{"line_number":31,"context_line":"        if not imports_ok:"},{"line_number":32,"context_line":"            return"},{"line_number":33,"context_line":"        self.order_meta \u003d {"},{"line_number":34,"context_line":"            \u0027cert_type\u0027: \u0027ssl123\u0027,"},{"line_number":35,"context_line":"            \u0027organization\u0027: \u0027Shinra Corp\u0027,"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_b137ebae","line":32,"updated":"2014-08-06 07:25:47.000000000","message":"instead of returning here, you could skip the whole test suit by replacing \"return\" for self.skip(\"\u003csome relevant text here\u003e\"). This way, you don\u0027t need to paste self.skipTest in every test like it is done at the moment. Thus, more cleanliness :D","commit_id":"0dfc4e6dc06dd3e5b9e16706d365df9e5dccf59f"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"ba1ed3137ed2276b1a94606f4c4077174c30c323","unresolved":false,"context_lines":[{"line_number":29,"context_line":"    def setUp(self):"},{"line_number":30,"context_line":"        super(WhenTestingSymantecPlugin, self).setUp()"},{"line_number":31,"context_line":"        if not imports_ok:"},{"line_number":32,"context_line":"            return"},{"line_number":33,"context_line":"        self.order_meta \u003d {"},{"line_number":34,"context_line":"            \u0027cert_type\u0027: \u0027ssl123\u0027,"},{"line_number":35,"context_line":"            \u0027organization\u0027: \u0027Shinra Corp\u0027,"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_c643b53d","line":32,"in_reply_to":"baada198_b137ebae","updated":"2014-08-06 15:18:03.000000000","message":"Nice! Other unit test modules (test_dogtag.py maybe?) could benefit from that too (in another CR of course :).","commit_id":"0dfc4e6dc06dd3e5b9e16706d365df9e5dccf59f"},{"author":{"_account_id":10873,"name":"Juan Antonio Osorio Robles","email":"jaosorior@redhat.com","username":"ejuaoso"},"change_message_id":"61102ca1adb1c8577d7be7af12347943a67963db","unresolved":false,"context_lines":[{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def test_successful_issue_certificate_request(self):"},{"line_number":54,"context_line":"        if not imports_ok:"},{"line_number":55,"context_line":"            self.skipTest(\"Symantec imports not available\")"},{"line_number":56,"context_line":"        self.mock_create_order.return_value \u003d (True, None, None)"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"        order_id \u003d \u00271234\u0027"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_3143db12","line":55,"updated":"2014-08-06 07:25:47.000000000","message":"So this would be removed","commit_id":"0dfc4e6dc06dd3e5b9e16706d365df9e5dccf59f"},{"author":{"_account_id":10873,"name":"Juan Antonio Osorio Robles","email":"jaosorior@redhat.com","username":"ejuaoso"},"change_message_id":"61102ca1adb1c8577d7be7af12347943a67963db","unresolved":false,"context_lines":[{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    def test_unsuccessful_certificate_request(self):"},{"line_number":70,"context_line":"        if not imports_ok:"},{"line_number":71,"context_line":"            self.skipTest(\"Symantec imports not available\")"},{"line_number":72,"context_line":"        self.mock_create_order.return_value \u003d (False, self.error_msg, True)"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        order_id \u003d \u00271234\u0027"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_714d533b","line":71,"updated":"2014-08-06 07:25:47.000000000","message":"And this","commit_id":"0dfc4e6dc06dd3e5b9e16706d365df9e5dccf59f"},{"author":{"_account_id":10873,"name":"Juan Antonio Osorio Robles","email":"jaosorior@redhat.com","username":"ejuaoso"},"change_message_id":"61102ca1adb1c8577d7be7af12347943a67963db","unresolved":false,"context_lines":[{"line_number":84,"context_line":""},{"line_number":85,"context_line":"    def test_should_raise_unsupported_certificate_request(self):"},{"line_number":86,"context_line":"        if not imports_ok:"},{"line_number":87,"context_line":"            self.skipTest(\"Symantec imports not available\")"},{"line_number":88,"context_line":"        order_id \u003d \u00271234\u0027"},{"line_number":89,"context_line":"        plugin_meta \u003d dict()"},{"line_number":90,"context_line":"        self.assertRaises("}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_91590703","line":87,"updated":"2014-08-06 07:25:47.000000000","message":"And this.","commit_id":"0dfc4e6dc06dd3e5b9e16706d365df9e5dccf59f"},{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"053a93a4c56ec90800a6d945e9fc4c71e5b3d4bf","unresolved":false,"context_lines":[{"line_number":20,"context_line":"    import barbican.plugin.symantec as sym"},{"line_number":21,"context_line":"    imports_ok \u003d True"},{"line_number":22,"context_line":"except ImportError:"},{"line_number":23,"context_line":"    # dogtag imports probably not available"},{"line_number":24,"context_line":"    imports_ok \u003d False"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"baada198_320d8790","line":23,"updated":"2014-08-06 17:03:49.000000000","message":"dogtag?","commit_id":"a01aa59efa45da781e99a7fc4aaa8d369398be27"},{"author":{"_account_id":11970,"name":"Chelsea Winfree","email":"chelsea.winfree@gmail.com","username":"chellygel"},"change_message_id":"b74c15f88a6fe772fdc24d0afb4dfb599e2905e2","unresolved":false,"context_lines":[{"line_number":20,"context_line":"    import barbican.plugin.symantec as sym"},{"line_number":21,"context_line":"    imports_ok \u003d True"},{"line_number":22,"context_line":"except ImportError:"},{"line_number":23,"context_line":"    # dogtag imports probably not available"},{"line_number":24,"context_line":"    imports_ok \u003d False"},{"line_number":25,"context_line":""},{"line_number":26,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"baada198_a61cfa67","line":23,"in_reply_to":"baada198_320d8790","updated":"2014-08-06 19:34:06.000000000","message":"whoopsies. fixed that.","commit_id":"a01aa59efa45da781e99a7fc4aaa8d369398be27"},{"author":{"_account_id":7973,"name":"Douglas Mendizábal","email":"dmendiza@redhat.com","username":"dougmendizabal"},"change_message_id":"45c62122484f2d7ea02d4621536ffe7bdbdb7a11","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    def setUp(self):"},{"line_number":31,"context_line":"        super(WhenTestingSymantecPlugin, self).setUp()"},{"line_number":32,"context_line":"        if not imports_ok:"},{"line_number":33,"context_line":"            self.skip(\"Symantec imports not available, test skipped\")"},{"line_number":34,"context_line":"        self.order_meta \u003d {"},{"line_number":35,"context_line":"            \u0027cert_type\u0027: \u0027ssl123\u0027,"},{"line_number":36,"context_line":"            \u0027organization\u0027: \u0027Shinra Corp\u0027,"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_bbaa41d1","line":33,"updated":"2014-08-07 05:35:25.000000000","message":"Why is this needed in addition to the class decorator?","commit_id":"144f1c0bf1adbbea6c056a46968fd68637ced0f7"},{"author":{"_account_id":10873,"name":"Juan Antonio Osorio Robles","email":"jaosorior@redhat.com","username":"ejuaoso"},"change_message_id":"2809d2b49fc86c1fd00bb7b410aa0a7dc59033c0","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    def setUp(self):"},{"line_number":31,"context_line":"        super(WhenTestingSymantecPlugin, self).setUp()"},{"line_number":32,"context_line":"        if not imports_ok:"},{"line_number":33,"context_line":"            self.skip(\"Symantec imports not available, test skipped\")"},{"line_number":34,"context_line":"        self.order_meta \u003d {"},{"line_number":35,"context_line":"            \u0027cert_type\u0027: \u0027ssl123\u0027,"},{"line_number":36,"context_line":"            \u0027organization\u0027: \u0027Shinra Corp\u0027,"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_e1508ad7","line":33,"in_reply_to":"baada198_bbaa41d1","updated":"2014-08-07 07:20:16.000000000","message":"++","commit_id":"144f1c0bf1adbbea6c056a46968fd68637ced0f7"}],"barbican/tests/tasks/test_certificate_resources.py":[{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"053a93a4c56ec90800a6d945e9fc4c71e5b3d4bf","unresolved":false,"context_lines":[{"line_number":99,"context_line":""},{"line_number":100,"context_line":"        self._verify_issue_certificate_plugins_called()"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    #TODO(chellygel): Will need to be renamed when else statement is written"},{"line_number":103,"context_line":"    def test_should_raise_not_implemented(self):"},{"line_number":104,"context_line":"        self.result.status \u003d \"Legend of Link\""},{"line_number":105,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"baada198_b21277fb","line":102,"updated":"2014-08-06 17:03:49.000000000","message":"would you be so kind as to clarify which \"else statement\" this refers to?  I assumed it was in certificate_resources.py but I don\u0027t see it.","commit_id":"a01aa59efa45da781e99a7fc4aaa8d369398be27"},{"author":{"_account_id":11970,"name":"Chelsea Winfree","email":"chelsea.winfree@gmail.com","username":"chellygel"},"change_message_id":"b74c15f88a6fe772fdc24d0afb4dfb599e2905e2","unresolved":false,"context_lines":[{"line_number":99,"context_line":""},{"line_number":100,"context_line":"        self._verify_issue_certificate_plugins_called()"},{"line_number":101,"context_line":""},{"line_number":102,"context_line":"    #TODO(chellygel): Will need to be renamed when else statement is written"},{"line_number":103,"context_line":"    def test_should_raise_not_implemented(self):"},{"line_number":104,"context_line":"        self.result.status \u003d \"Legend of Link\""},{"line_number":105,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"baada198_867376b2","line":102,"in_reply_to":"baada198_b21277fb","updated":"2014-08-06 19:34:06.000000000","message":"thanks, removing this and updating the name.","commit_id":"a01aa59efa45da781e99a7fc4aaa8d369398be27"}],"barbican/tests/tasks/test_certificates.py":[{"author":{"_account_id":9914,"name":"Ade Lee","email":"alee@redhat.com","username":"alee"},"change_message_id":"4a630fe418fc97e9e820fab8330687ecf6b0d7fe","unresolved":false,"context_lines":[{"line_number":17,"context_line":"import testtools"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"from barbican.plugin.interface import certificates as icerts"},{"line_number":20,"context_line":"from barbican.tasks import certificates as tcerts"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"class WhenIssuingCertificateRequests(testtools.TestCase):"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_0764ebe4","line":20,"updated":"2014-07-29 15:51:30.000000000","message":"icerts and tcerts are not very distinguishable.  Perhaps it would be better to rename plugin/interface/certificates.py to be certificate_issuer.py to be more in line with secret_store.py?\n\nFor the tasks file, I\u0027m not totally convinced that certificates.py is the best name, but its probably better to reference it here as cert_tasks rather than tcerts","commit_id":"ee58fe1f0007b4f332db184db52f4a7ce5ddb05d"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"bd817efda92d687f99f014a04bbb2d76d43f454c","unresolved":false,"context_lines":[{"line_number":17,"context_line":"import testtools"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"from barbican.plugin.interface import certificates as icerts"},{"line_number":20,"context_line":"from barbican.tasks import certificates as tcerts"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"class WhenIssuingCertificateRequests(testtools.TestCase):"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_0f3939cf","line":20,"in_reply_to":"baada198_0764ebe4","updated":"2014-07-30 14:01:05.000000000","message":"To me certificate_issuer.py implies barbican are acting as the certificate issuer/authority. Maybe certificate_manager.py then? I\u0027d expect to have cert revoke/renew capabilities in here as well at some point? At any rate, I\u0027m thinking that would be a separate CR.\n\nAs for the tasks/certificates.py module, for consistency with other packages it would probably be better to have this in a tasks/resources.py module, but that is \u0027taken\u0027 now with the task implementing classes there now, so such change should be made with another CR I\u0027d think.","commit_id":"ee58fe1f0007b4f332db184db52f4a7ce5ddb05d"},{"author":{"_account_id":9914,"name":"Ade Lee","email":"alee@redhat.com","username":"alee"},"change_message_id":"e2135a16efab9069c4c2a4e5f6aba80cfc9c13c1","unresolved":false,"context_lines":[{"line_number":17,"context_line":"import testtools"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"from barbican.plugin.interface import certificates as icerts"},{"line_number":20,"context_line":"from barbican.tasks import certificates as tcerts"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"class WhenIssuingCertificateRequests(testtools.TestCase):"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_f5f4d289","line":20,"in_reply_to":"baada198_0f3939cf","updated":"2014-07-30 14:58:41.000000000","message":"I\u0027m ok with certificate_manager.py.  Why not make that change in this CR?  These are brand new files.\n\nI\u0027m not sure there is any reason for it to necessarily be in tasks/resources.py.  If we put all tasks in that file, it will get large rather quickly.\n\nWhat about tasks/certificate_resources.py ?","commit_id":"ee58fe1f0007b4f332db184db52f4a7ce5ddb05d"},{"author":{"_account_id":9914,"name":"Ade Lee","email":"alee@redhat.com","username":"alee"},"change_message_id":"4a630fe418fc97e9e820fab8330687ecf6b0d7fe","unresolved":false,"context_lines":[{"line_number":107,"context_line":"        )"},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"    def _verify_plugin_called(self):"},{"line_number":110,"context_line":"        self.cert_plugin.issue_certificate_request.assert_called_once_with("},{"line_number":111,"context_line":"            self.order_id,"},{"line_number":112,"context_line":"            self.order_meta,"},{"line_number":113,"context_line":"            self.plugin_meta"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_c7959333","line":110,"range":{"start_line":110,"start_character":0,"end_line":110,"end_character":8},"updated":"2014-07-29 15:51:30.000000000","message":"You probably want to rename this method to something more specific to cert issuance.  We\u0027ll be adding tests for cancellation etc.","commit_id":"ee58fe1f0007b4f332db184db52f4a7ce5ddb05d"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"bd817efda92d687f99f014a04bbb2d76d43f454c","unresolved":false,"context_lines":[{"line_number":107,"context_line":"        )"},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"    def _verify_plugin_called(self):"},{"line_number":110,"context_line":"        self.cert_plugin.issue_certificate_request.assert_called_once_with("},{"line_number":111,"context_line":"            self.order_id,"},{"line_number":112,"context_line":"            self.order_meta,"},{"line_number":113,"context_line":"            self.plugin_meta"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_efab25d5","line":110,"range":{"start_line":110,"start_character":0,"end_line":110,"end_character":8},"in_reply_to":"baada198_c7959333","updated":"2014-07-30 14:01:05.000000000","message":"Like _verify_issue_certificate_plugins_called() maybe?","commit_id":"ee58fe1f0007b4f332db184db52f4a7ce5ddb05d"},{"author":{"_account_id":9914,"name":"Ade Lee","email":"alee@redhat.com","username":"alee"},"change_message_id":"e2135a16efab9069c4c2a4e5f6aba80cfc9c13c1","unresolved":false,"context_lines":[{"line_number":107,"context_line":"        )"},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"    def _verify_plugin_called(self):"},{"line_number":110,"context_line":"        self.cert_plugin.issue_certificate_request.assert_called_once_with("},{"line_number":111,"context_line":"            self.order_id,"},{"line_number":112,"context_line":"            self.order_meta,"},{"line_number":113,"context_line":"            self.plugin_meta"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_55973e45","line":110,"range":{"start_line":110,"start_character":0,"end_line":110,"end_character":8},"in_reply_to":"baada198_efab25d5","updated":"2014-07-30 14:58:41.000000000","message":"sure. that sounds ok.","commit_id":"ee58fe1f0007b4f332db184db52f4a7ce5ddb05d"}]}