)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"f5ba9b8ef03821840e236e35a06421d40becaa22","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"First attempt at adding the symantecssl library"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Attempted to implement the functions to create,"},{"line_number":10,"context_line":"modify, check status, and cancel certificate orders"},{"line_number":11,"context_line":"using the symantecssl library."},{"line_number":12,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"baada198_206fe7fb","line":9,"updated":"2014-07-30 16:51:47.000000000","message":"I would get rid of \"Attempted to\".  You DID implement :-)","commit_id":"197ce5856be2cdfafb3b0492e99bce777867b57d"}],"barbican/plugin/symantec.py":[{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"f5ba9b8ef03821840e236e35a06421d40becaa22","unresolved":false,"context_lines":[{"line_number":17,"context_line":"Barbican certificate processing plugins and support."},{"line_number":18,"context_line":"\"\"\""},{"line_number":19,"context_line":"from symantecssl import Symantec"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"from barbican.plugin.interface import certificates as cert"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_20044718","line":20,"updated":"2014-07-30 16:51:47.000000000","message":"does this need to go into requirements.txt?","commit_id":"197ce5856be2cdfafb3b0492e99bce777867b57d"},{"author":{"_account_id":11661,"name":"Kevin Bishop","email":"ktb2012@att.net","username":"kbishop711"},"change_message_id":"92667e17957520f4adea44d91ebe09b127a946b8","unresolved":false,"context_lines":[{"line_number":17,"context_line":"Barbican certificate processing plugins and support."},{"line_number":18,"context_line":"\"\"\""},{"line_number":19,"context_line":"from symantecssl import Symantec"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"from barbican.plugin.interface import certificates as cert"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_b10507c3","line":20,"in_reply_to":"baada198_20044718","updated":"2014-07-30 17:42:10.000000000","message":"I don\u0027t think so. Currently the testing for the plugin is done such that if this plugin fails to import everything, then the tests are skipped. I believe the other plugins like Dogtag do it this way as well.","commit_id":"197ce5856be2cdfafb3b0492e99bce777867b57d"},{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"f5ba9b8ef03821840e236e35a06421d40becaa22","unresolved":false,"context_lines":[{"line_number":89,"context_line":"    \"\"\""},{"line_number":90,"context_line":"    #TODO(jwood) Submit order to CA, get partner order id and"},{"line_number":91,"context_line":"    #   then add to the plugin meta data."},{"line_number":92,"context_line":"    "},{"line_number":93,"context_line":"    api \u003d Symantec(plugin_meta[\"username\"], plugin_meta[\"password\"], plugin_meta[\"url\"])"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"    \"\"\"Needs several args from either order_meta or plugin_meta:"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_3b0b9823","line":92,"updated":"2014-07-30 16:51:47.000000000","message":"zap the spaces since Gerrit is finicky","commit_id":"197ce5856be2cdfafb3b0492e99bce777867b57d"},{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"f5ba9b8ef03821840e236e35a06421d40becaa22","unresolved":false,"context_lines":[{"line_number":101,"context_line":"    city"},{"line_number":102,"context_line":"    region"},{"line_number":103,"context_line":"    postalcode"},{"line_number":104,"context_line":"    counter "},{"line_number":105,"context_line":"    cont."},{"line_number":106,"context_line":"    \"\"\""},{"line_number":107,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_db061c1c","line":104,"updated":"2014-07-30 16:51:47.000000000","message":"zap the trailing space since Gerrit is still finicky","commit_id":"197ce5856be2cdfafb3b0492e99bce777867b57d"},{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"f5ba9b8ef03821840e236e35a06421d40becaa22","unresolved":false,"context_lines":[{"line_number":138,"context_line":"        )"},{"line_number":139,"context_line":"        plugin_meta[\"GeotrustOrderID\"] \u003d order_data[\"GeotrustOrderID\"]"},{"line_number":140,"context_line":"        plugin_meta[\"PartnerOrderID\"] \u003d order_data[\"PartnerOrderID\"]"},{"line_number":141,"context_line":"        return True, None, None"},{"line_number":142,"context_line":"    except SymantecError as e:"},{"line_number":143,"context_line":"        # Maybe be able to retry on error code -1001"},{"line_number":144,"context_line":"        return False, e, False"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_3b39d8cf","line":141,"updated":"2014-07-30 16:51:47.000000000","message":"canRetry should be False instead of None?  I know that it previously used None but seems that if its a boolean it should be explicitly False.  Just my 2 euros...","commit_id":"197ce5856be2cdfafb3b0492e99bce777867b57d"},{"author":{"_account_id":11661,"name":"Kevin Bishop","email":"ktb2012@att.net","username":"kbishop711"},"change_message_id":"92667e17957520f4adea44d91ebe09b127a946b8","unresolved":false,"context_lines":[{"line_number":138,"context_line":"        )"},{"line_number":139,"context_line":"        plugin_meta[\"GeotrustOrderID\"] \u003d order_data[\"GeotrustOrderID\"]"},{"line_number":140,"context_line":"        plugin_meta[\"PartnerOrderID\"] \u003d order_data[\"PartnerOrderID\"]"},{"line_number":141,"context_line":"        return True, None, None"},{"line_number":142,"context_line":"    except SymantecError as e:"},{"line_number":143,"context_line":"        # Maybe be able to retry on error code -1001"},{"line_number":144,"context_line":"        return False, e, False"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_f10f7fde","line":141,"in_reply_to":"baada198_3b39d8cf","updated":"2014-07-30 17:42:10.000000000","message":"Sound reasonable to me.","commit_id":"197ce5856be2cdfafb3b0492e99bce777867b57d"},{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"f5ba9b8ef03821840e236e35a06421d40becaa22","unresolved":false,"context_lines":[{"line_number":143,"context_line":"        # Maybe be able to retry on error code -1001"},{"line_number":144,"context_line":"        return False, e, False"},{"line_number":145,"context_line":"    # Some kind of exception with canretry \u003d True can go here?"},{"line_number":146,"context_line":"    except:"},{"line_number":147,"context_line":"        return False, sys.exc_info()[0], False"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_7b88b06b","line":146,"updated":"2014-07-30 16:51:47.000000000","message":"bare excepts aren\u0027t a very good idea - could end up catching things that we shouldn\u0027t.","commit_id":"197ce5856be2cdfafb3b0492e99bce777867b57d"},{"author":{"_account_id":11661,"name":"Kevin Bishop","email":"ktb2012@att.net","username":"kbishop711"},"change_message_id":"92667e17957520f4adea44d91ebe09b127a946b8","unresolved":false,"context_lines":[{"line_number":143,"context_line":"        # Maybe be able to retry on error code -1001"},{"line_number":144,"context_line":"        return False, e, False"},{"line_number":145,"context_line":"    # Some kind of exception with canretry \u003d True can go here?"},{"line_number":146,"context_line":"    except:"},{"line_number":147,"context_line":"        return False, sys.exc_info()[0], False"},{"line_number":148,"context_line":""},{"line_number":149,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_111dd310","line":146,"in_reply_to":"baada198_7b88b06b","updated":"2014-07-30 17:42:10.000000000","message":"Ok, I can take this part out. Is it ok to just have the \"except SymantecError\" though in case of something else going wrong?","commit_id":"197ce5856be2cdfafb3b0492e99bce777867b57d"},{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"f5ba9b8ef03821840e236e35a06421d40becaa22","unresolved":false,"context_lines":[{"line_number":148,"context_line":""},{"line_number":149,"context_line":""},{"line_number":150,"context_line":"def _ca_get_order_status(self, order_meta, plugin_meta):"},{"line_number":151,"context_line":"    \"\"\""},{"line_number":152,"context_line":"    INFO NEEDED FOR getOrderByPartnerOrderID:"},{"line_number":153,"context_line":"    partnerOrderId"},{"line_number":154,"context_line":"    partnerCode"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_9be66449","line":151,"updated":"2014-07-30 16:51:47.000000000","message":"A bit more description here would be cool, especially :returns for the values its sending back","commit_id":"197ce5856be2cdfafb3b0492e99bce777867b57d"},{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"f5ba9b8ef03821840e236e35a06421d40becaa22","unresolved":false,"context_lines":[{"line_number":164,"context_line":"        return False, e, False"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"def _ca_modify_order(self, order_meta, plugin_meta):"},{"line_number":168,"context_line":"    \"\"\""},{"line_number":169,"context_line":"    INFO NEEDED FOR modifyOrder:"},{"line_number":170,"context_line":"    partnerorderid"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_9bcd84bd","line":167,"updated":"2014-07-30 16:51:47.000000000","message":"same as above - more description and :returns","commit_id":"197ce5856be2cdfafb3b0492e99bce777867b57d"},{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"f5ba9b8ef03821840e236e35a06421d40becaa22","unresolved":false,"context_lines":[{"line_number":177,"context_line":"        order_data \u003d api.modify_order("},{"line_number":178,"context_line":"            partnerorderid\u003dplugin_meta[\"PluginOrderID\"],"},{"line_number":179,"context_line":"            partnercode\u003dplugin_meta[\"PartnerCode\"],"},{"line_number":180,"context_line":"            productcode\u003dorder_meta[\"productcode\"],"},{"line_number":181,"context_line":"            modifyorderoperation\u003dorder_meta[\"modifyorderoperation\"],"},{"line_number":182,"context_line":"        )"},{"line_number":183,"context_line":"        return True, None, None"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_fb0f60de","line":180,"updated":"2014-07-30 16:51:47.000000000","message":"why isn\u0027t productcode camel case?","commit_id":"197ce5856be2cdfafb3b0492e99bce777867b57d"},{"author":{"_account_id":11661,"name":"Kevin Bishop","email":"ktb2012@att.net","username":"kbishop711"},"change_message_id":"92667e17957520f4adea44d91ebe09b127a946b8","unresolved":false,"context_lines":[{"line_number":177,"context_line":"        order_data \u003d api.modify_order("},{"line_number":178,"context_line":"            partnerorderid\u003dplugin_meta[\"PluginOrderID\"],"},{"line_number":179,"context_line":"            partnercode\u003dplugin_meta[\"PartnerCode\"],"},{"line_number":180,"context_line":"            productcode\u003dorder_meta[\"productcode\"],"},{"line_number":181,"context_line":"            modifyorderoperation\u003dorder_meta[\"modifyorderoperation\"],"},{"line_number":182,"context_line":"        )"},{"line_number":183,"context_line":"        return True, None, None"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_3118171f","line":180,"in_reply_to":"baada198_fb0f60de","updated":"2014-07-30 17:42:10.000000000","message":"Oops, my mistake, though most of the names are assumptions on my end because they need to be passed in plugin-meta, order_meta.","commit_id":"197ce5856be2cdfafb3b0492e99bce777867b57d"},{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"f5ba9b8ef03821840e236e35a06421d40becaa22","unresolved":false,"context_lines":[{"line_number":178,"context_line":"            partnerorderid\u003dplugin_meta[\"PluginOrderID\"],"},{"line_number":179,"context_line":"            partnercode\u003dplugin_meta[\"PartnerCode\"],"},{"line_number":180,"context_line":"            productcode\u003dorder_meta[\"productcode\"],"},{"line_number":181,"context_line":"            modifyorderoperation\u003dorder_meta[\"modifyorderoperation\"],"},{"line_number":182,"context_line":"        )"},{"line_number":183,"context_line":"        return True, None, None"},{"line_number":184,"context_line":"    except SymantecError as e:"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_5b004cab","line":181,"updated":"2014-07-30 16:51:47.000000000","message":"ditto for modifyorderoperation - camel case?","commit_id":"197ce5856be2cdfafb3b0492e99bce777867b57d"},{"author":{"_account_id":11661,"name":"Kevin Bishop","email":"ktb2012@att.net","username":"kbishop711"},"change_message_id":"92667e17957520f4adea44d91ebe09b127a946b8","unresolved":false,"context_lines":[{"line_number":178,"context_line":"            partnerorderid\u003dplugin_meta[\"PluginOrderID\"],"},{"line_number":179,"context_line":"            partnercode\u003dplugin_meta[\"PartnerCode\"],"},{"line_number":180,"context_line":"            productcode\u003dorder_meta[\"productcode\"],"},{"line_number":181,"context_line":"            modifyorderoperation\u003dorder_meta[\"modifyorderoperation\"],"},{"line_number":182,"context_line":"        )"},{"line_number":183,"context_line":"        return True, None, None"},{"line_number":184,"context_line":"    except SymantecError as e:"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_51174bee","line":181,"in_reply_to":"baada198_5b004cab","updated":"2014-07-30 17:42:10.000000000","message":"Ditto","commit_id":"197ce5856be2cdfafb3b0492e99bce777867b57d"},{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"f5ba9b8ef03821840e236e35a06421d40becaa22","unresolved":false,"context_lines":[{"line_number":185,"context_line":"        return False, e, False"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"def _ca_cancel_order(self, order_meta, plugin_meta):"},{"line_number":189,"context_line":"    \"\"\""},{"line_number":190,"context_line":"    INFO NEEDED FOR modifyOrder when cancelling:"},{"line_number":191,"context_line":"    partnerorderid"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_bbd08853","line":188,"updated":"2014-07-30 16:51:47.000000000","message":"same as above and above above...more description and :returns","commit_id":"197ce5856be2cdfafb3b0492e99bce777867b57d"},{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"f5ba9b8ef03821840e236e35a06421d40becaa22","unresolved":false,"context_lines":[{"line_number":186,"context_line":""},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"def _ca_cancel_order(self, order_meta, plugin_meta):"},{"line_number":189,"context_line":"    \"\"\""},{"line_number":190,"context_line":"    INFO NEEDED FOR modifyOrder when cancelling:"},{"line_number":191,"context_line":"    partnerorderid"},{"line_number":192,"context_line":"    partnercode"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_5b692c8b","line":189,"updated":"2014-07-30 16:51:47.000000000","message":"This sure looks like 99% of modify order code.  Since we are very DRY here (Don\u0027t Repeat Yourself) I will suggest making this a call into modify with order_meta[modifyoperation\u0027] set to cancel","commit_id":"197ce5856be2cdfafb3b0492e99bce777867b57d"},{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"69524dceb07387db3a8a2ba1cbeb9e07b4c596e3","unresolved":false,"context_lines":[{"line_number":83,"context_line":""},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"def _ca_create_order(self, order_meta, plugin_meta):"},{"line_number":86,"context_line":"    \"\"\"Creates an order with the Symantec CA."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"    :returns: tuple with success, error message, and can retry"},{"line_number":89,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"baada198_3537f9e5","line":86,"updated":"2014-07-31 19:14:33.000000000","message":"would be good to note the fields in the metadata that get updated as a result of the order creation.","commit_id":"f87f1efa76012f6a15ad3ae3af4e901b61fc8fcd"},{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"69524dceb07387db3a8a2ba1cbeb9e07b4c596e3","unresolved":false,"context_lines":[{"line_number":96,"context_line":"    try:"},{"line_number":97,"context_line":"        # (kevin_bishop): These order parameters need to come from either"},{"line_number":98,"context_line":"        # order_meta or plugin_meta, but I\u0027m not sure which parameters"},{"line_number":99,"context_line":"        # come from where."},{"line_number":100,"context_line":"        order_data \u003d api.order("},{"line_number":101,"context_line":"            partnercode\u003dorder_meta[\"PartnerCode\"],"},{"line_number":102,"context_line":"            productcode\u003dorder_meta[\"ProductCode\"],"}],"source_content_type":"text/x-python","patch_set":5,"id":"baada198_23134666","line":99,"updated":"2014-07-31 19:14:33.000000000","message":"How do we figure out what comes from where?  Or will that be a follow-on CR?","commit_id":"f87f1efa76012f6a15ad3ae3af4e901b61fc8fcd"},{"author":{"_account_id":11661,"name":"Kevin Bishop","email":"ktb2012@att.net","username":"kbishop711"},"change_message_id":"1603a510bcc29a5b4925baad2396c18256a1fda0","unresolved":false,"context_lines":[{"line_number":96,"context_line":"    try:"},{"line_number":97,"context_line":"        # (kevin_bishop): These order parameters need to come from either"},{"line_number":98,"context_line":"        # order_meta or plugin_meta, but I\u0027m not sure which parameters"},{"line_number":99,"context_line":"        # come from where."},{"line_number":100,"context_line":"        order_data \u003d api.order("},{"line_number":101,"context_line":"            partnercode\u003dorder_meta[\"PartnerCode\"],"},{"line_number":102,"context_line":"            productcode\u003dorder_meta[\"ProductCode\"],"}],"source_content_type":"text/x-python","patch_set":5,"id":"baada198_3f8192a0","line":99,"in_reply_to":"baada198_23134666","updated":"2014-07-31 19:25:24.000000000","message":"I\u0027ve currently been looking for information on this. It should be in this CR.","commit_id":"f87f1efa76012f6a15ad3ae3af4e901b61fc8fcd"},{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"69524dceb07387db3a8a2ba1cbeb9e07b4c596e3","unresolved":false,"context_lines":[{"line_number":100,"context_line":"        order_data \u003d api.order("},{"line_number":101,"context_line":"            partnercode\u003dorder_meta[\"PartnerCode\"],"},{"line_number":102,"context_line":"            productcode\u003dorder_meta[\"ProductCode\"],"},{"line_number":103,"context_line":"            partnerorderid\u003dorder_meta[\"PartnerOrderId\"],"},{"line_number":104,"context_line":"            organizationname\u003dorder_meta[\"OrganizationName\"],"},{"line_number":105,"context_line":"            addressline1\u003dorder_meta[\"AddressLine1\"],"},{"line_number":106,"context_line":"            city\u003dorder_meta[\"City\"],"}],"source_content_type":"text/x-python","patch_set":5,"id":"baada198_d574fd9c","line":103,"updated":"2014-07-31 19:14:33.000000000","message":"there is an order id provided for input?  It looks like it just gets generated for output.","commit_id":"f87f1efa76012f6a15ad3ae3af4e901b61fc8fcd"},{"author":{"_account_id":11661,"name":"Kevin Bishop","email":"ktb2012@att.net","username":"kbishop711"},"change_message_id":"1603a510bcc29a5b4925baad2396c18256a1fda0","unresolved":false,"context_lines":[{"line_number":100,"context_line":"        order_data \u003d api.order("},{"line_number":101,"context_line":"            partnercode\u003dorder_meta[\"PartnerCode\"],"},{"line_number":102,"context_line":"            productcode\u003dorder_meta[\"ProductCode\"],"},{"line_number":103,"context_line":"            partnerorderid\u003dorder_meta[\"PartnerOrderId\"],"},{"line_number":104,"context_line":"            organizationname\u003dorder_meta[\"OrganizationName\"],"},{"line_number":105,"context_line":"            addressline1\u003dorder_meta[\"AddressLine1\"],"},{"line_number":106,"context_line":"            city\u003dorder_meta[\"City\"],"}],"source_content_type":"text/x-python","patch_set":5,"id":"baada198_5fc5665a","line":103,"in_reply_to":"baada198_d574fd9c","updated":"2014-07-31 19:25:24.000000000","message":"You can specify what you want the returned PartnerOrderId to be when the order is created. If you don\u0027t specify, then it is randomly generated.","commit_id":"f87f1efa76012f6a15ad3ae3af4e901b61fc8fcd"},{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"69524dceb07387db3a8a2ba1cbeb9e07b4c596e3","unresolved":false,"context_lines":[{"line_number":102,"context_line":"            productcode\u003dorder_meta[\"ProductCode\"],"},{"line_number":103,"context_line":"            partnerorderid\u003dorder_meta[\"PartnerOrderId\"],"},{"line_number":104,"context_line":"            organizationname\u003dorder_meta[\"OrganizationName\"],"},{"line_number":105,"context_line":"            addressline1\u003dorder_meta[\"AddressLine1\"],"},{"line_number":106,"context_line":"            city\u003dorder_meta[\"City\"],"},{"line_number":107,"context_line":"            region\u003dorder_meta[\"Region\"],"},{"line_number":108,"context_line":"            postalcode\u003dorder_meta[\"PostalCode\"],"}],"source_content_type":"text/x-python","patch_set":5,"id":"baada198_b5cdc9bc","line":105,"updated":"2014-07-31 19:14:33.000000000","message":"are there more than 1 address lines?  Do the all need to be sent, or just the first?","commit_id":"f87f1efa76012f6a15ad3ae3af4e901b61fc8fcd"},{"author":{"_account_id":11661,"name":"Kevin Bishop","email":"ktb2012@att.net","username":"kbishop711"},"change_message_id":"1603a510bcc29a5b4925baad2396c18256a1fda0","unresolved":false,"context_lines":[{"line_number":102,"context_line":"            productcode\u003dorder_meta[\"ProductCode\"],"},{"line_number":103,"context_line":"            partnerorderid\u003dorder_meta[\"PartnerOrderId\"],"},{"line_number":104,"context_line":"            organizationname\u003dorder_meta[\"OrganizationName\"],"},{"line_number":105,"context_line":"            addressline1\u003dorder_meta[\"AddressLine1\"],"},{"line_number":106,"context_line":"            city\u003dorder_meta[\"City\"],"},{"line_number":107,"context_line":"            region\u003dorder_meta[\"Region\"],"},{"line_number":108,"context_line":"            postalcode\u003dorder_meta[\"PostalCode\"],"}],"source_content_type":"text/x-python","patch_set":5,"id":"baada198_7fc06a4a","line":105,"in_reply_to":"baada198_b5cdc9bc","updated":"2014-07-31 19:25:24.000000000","message":"You only need to send the first one, but there are 2.","commit_id":"f87f1efa76012f6a15ad3ae3af4e901b61fc8fcd"},{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"69524dceb07387db3a8a2ba1cbeb9e07b4c596e3","unresolved":false,"context_lines":[{"line_number":139,"context_line":""},{"line_number":140,"context_line":"def _ca_get_order_status(self, order_meta, plugin_meta):"},{"line_number":141,"context_line":"    \"\"\"Sends a request to the Symantec CA for details on an order."},{"line_number":142,"context_line":"    Parameters needed for getOrderByPartnerOrderID:"},{"line_number":143,"context_line":"        partnerOrderId"},{"line_number":144,"context_line":"        partnerCode"},{"line_number":145,"context_line":"    Additional parameters can be added to return additional details."}],"source_content_type":"text/x-python","patch_set":5,"id":"baada198_ff41da0a","line":142,"updated":"2014-07-31 19:14:33.000000000","message":"if this is the api name then should probably have match (ie not camel case)","commit_id":"f87f1efa76012f6a15ad3ae3af4e901b61fc8fcd"},{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"69524dceb07387db3a8a2ba1cbeb9e07b4c596e3","unresolved":false,"context_lines":[{"line_number":144,"context_line":"        partnerCode"},{"line_number":145,"context_line":"    Additional parameters can be added to return additional details."},{"line_number":146,"context_line":"    If the order is complete, the Certificate is also returned in the details."},{"line_number":147,"context_line":"    returns: tuple with success, error message, can retry,"},{"line_number":148,"context_line":"             and a dictionary of details."},{"line_number":149,"context_line":"    \"\"\""},{"line_number":150,"context_line":"    api \u003d Symantec(plugin_meta[\"username\"], plugin_meta[\"password\"],"}],"source_content_type":"text/x-python","patch_set":5,"id":"baada198_bf5e82e6","line":147,"updated":"2014-07-31 19:14:33.000000000","message":"what determines if you can retry or not?  For this order status call we are always returning False for can_retry.  Is there ever a case where we want to return True?","commit_id":"f87f1efa76012f6a15ad3ae3af4e901b61fc8fcd"},{"author":{"_account_id":11661,"name":"Kevin Bishop","email":"ktb2012@att.net","username":"kbishop711"},"change_message_id":"1603a510bcc29a5b4925baad2396c18256a1fda0","unresolved":false,"context_lines":[{"line_number":144,"context_line":"        partnerCode"},{"line_number":145,"context_line":"    Additional parameters can be added to return additional details."},{"line_number":146,"context_line":"    If the order is complete, the Certificate is also returned in the details."},{"line_number":147,"context_line":"    returns: tuple with success, error message, can retry,"},{"line_number":148,"context_line":"             and a dictionary of details."},{"line_number":149,"context_line":"    \"\"\""},{"line_number":150,"context_line":"    api \u003d Symantec(plugin_meta[\"username\"], plugin_meta[\"password\"],"}],"source_content_type":"text/x-python","patch_set":5,"id":"baada198_3f651249","line":147,"in_reply_to":"baada198_bf5e82e6","updated":"2014-07-31 19:25:24.000000000","message":"According to the API docs, virtually all Symantec side errors are because one of the parameters was wrong, therefore retrying shouldn\u0027t be possible in that case. I think retry is there for other kinds of errors like maybe a connection interrupt?","commit_id":"f87f1efa76012f6a15ad3ae3af4e901b61fc8fcd"},{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"69524dceb07387db3a8a2ba1cbeb9e07b4c596e3","unresolved":false,"context_lines":[{"line_number":168,"context_line":"        partnerorderid"},{"line_number":169,"context_line":"        partnercode"},{"line_number":170,"context_line":"        productcode"},{"line_number":171,"context_line":"        modifyorderoperation"},{"line_number":172,"context_line":"    returns: tuple with success, error message, and can retry."},{"line_number":173,"context_line":"    \"\"\""},{"line_number":174,"context_line":"    api \u003d Symantec(plugin_meta[\"username\"], plugin_meta[\"password\"],"}],"source_content_type":"text/x-python","patch_set":5,"id":"baada198_5f1726ee","line":171,"updated":"2014-07-31 19:14:33.000000000","message":"do we modify all of these values?  What if caller only wants to update partnercode - do they still need to provide all of the others?","commit_id":"f87f1efa76012f6a15ad3ae3af4e901b61fc8fcd"},{"author":{"_account_id":11661,"name":"Kevin Bishop","email":"ktb2012@att.net","username":"kbishop711"},"change_message_id":"1603a510bcc29a5b4925baad2396c18256a1fda0","unresolved":false,"context_lines":[{"line_number":168,"context_line":"        partnerorderid"},{"line_number":169,"context_line":"        partnercode"},{"line_number":170,"context_line":"        productcode"},{"line_number":171,"context_line":"        modifyorderoperation"},{"line_number":172,"context_line":"    returns: tuple with success, error message, and can retry."},{"line_number":173,"context_line":"    \"\"\""},{"line_number":174,"context_line":"    api \u003d Symantec(plugin_meta[\"username\"], plugin_meta[\"password\"],"}],"source_content_type":"text/x-python","patch_set":5,"id":"baada198_df097685","line":171,"in_reply_to":"baada198_5f1726ee","updated":"2014-07-31 19:25:24.000000000","message":"the order id, partner code, and product code are how you specify which order you are modifying. The modify order operation is how you want to modify the order. Then you need to pass the more params based off what modify operation you\u0027re doing.","commit_id":"f87f1efa76012f6a15ad3ae3af4e901b61fc8fcd"},{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"69524dceb07387db3a8a2ba1cbeb9e07b4c596e3","unresolved":false,"context_lines":[{"line_number":169,"context_line":"        partnercode"},{"line_number":170,"context_line":"        productcode"},{"line_number":171,"context_line":"        modifyorderoperation"},{"line_number":172,"context_line":"    returns: tuple with success, error message, and can retry."},{"line_number":173,"context_line":"    \"\"\""},{"line_number":174,"context_line":"    api \u003d Symantec(plugin_meta[\"username\"], plugin_meta[\"password\"],"},{"line_number":175,"context_line":"                   plugin_meta[\"url\"])"}],"source_content_type":"text/x-python","patch_set":5,"id":"baada198_9f029eb8","line":172,"updated":"2014-07-31 19:14:33.000000000","message":"same comment as above about can_retry always returns False","commit_id":"f87f1efa76012f6a15ad3ae3af4e901b61fc8fcd"},{"author":{"_account_id":11970,"name":"Chelsea Winfree","email":"chelsea.winfree@gmail.com","username":"chellygel"},"change_message_id":"80d0272f0c9cc10b74c0f3c39c86f27b60d934fc","unresolved":false,"context_lines":[{"line_number":17,"context_line":"Barbican certificate processing plugins and support."},{"line_number":18,"context_line":"\"\"\""},{"line_number":19,"context_line":"from symantecssl import Symantec"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"from barbican.plugin.interface import certificates as cert"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"class SymantecCertificatePlugin(cert.CertificatePluginBase):"}],"source_content_type":"text/x-python","patch_set":6,"id":"baada198_2b9e8eb8","line":21,"range":{"start_line":20,"start_character":0,"end_line":21,"end_character":58},"updated":"2014-08-01 18:10:08.000000000","message":"i know i *JUST* submitted it, but this is changing to certificate_manager instead of certificates","commit_id":"6291fdec6e89be6ebb8904ebb0af536c80ba2542"},{"author":{"_account_id":11970,"name":"Chelsea Winfree","email":"chelsea.winfree@gmail.com","username":"chellygel"},"change_message_id":"80d0272f0c9cc10b74c0f3c39c86f27b60d934fc","unresolved":false,"context_lines":[{"line_number":83,"context_line":""},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"def _ca_create_order(self, order_meta, plugin_meta):"},{"line_number":86,"context_line":"    \"\"\"Creates an order with the Symantec CA. The PartnerOrderId "},{"line_number":87,"context_line":"    and GeoTrustOrderId are returned and stored in plugin_meta."},{"line_number":88,"context_line":""},{"line_number":89,"context_line":"    :returns: tuple with success, error message, and can retry"}],"source_content_type":"text/x-python","patch_set":6,"id":"baada198_4b6822bc","line":86,"updated":"2014-08-01 18:10:08.000000000","message":"whitespace at the end of the line here :S","commit_id":"6291fdec6e89be6ebb8904ebb0af536c80ba2542"},{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"94caf58f8aebf83169ba70161eb6f6ef3e859a05","unresolved":false,"context_lines":[{"line_number":16,"context_line":"\"\"\""},{"line_number":17,"context_line":"Barbican certificate processing plugins and support."},{"line_number":18,"context_line":"\"\"\""},{"line_number":19,"context_line":"from symantecssl import Symantec"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"from barbican.plugin.interface import certificate_manager as cert"},{"line_number":22,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"baada198_e73bd3c5","line":19,"updated":"2014-08-02 23:37:25.000000000","message":"tox tests are failing - can\u0027t find symantecssl.  Are you sure it doesn\u0027t have to be in requirements or test-requirements?","commit_id":"a98bf24dbb54a9495d627799455710867921473b"},{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"cb07d330e751424ffaf4f1f09e3b179fc414334f","unresolved":false,"context_lines":[{"line_number":173,"context_line":"        \"returncacerts\": \"TRUE\","},{"line_number":174,"context_line":"    }"},{"line_number":175,"context_line":""},{"line_number":176,"context_line":"    for key, value in order_meta[\"OrderStatus\"]:"},{"line_number":177,"context_line":"        order_details[key] \u003d value"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":12,"id":"baada198_866eeffc","line":176,"updated":"2014-08-07 18:03:48.000000000","message":"same logic as code up on line 130 just with different input dicts.  Should make it a function - something like populate(dest, source) or something equally silly.","commit_id":"33bbf3170890ec4065059381a3ba8c185cd72650"},{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"cb07d330e751424ffaf4f1f09e3b179fc414334f","unresolved":false,"context_lines":[{"line_number":208,"context_line":"    }"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"    for key, value in order_meta[\"ModifyDetails\"]:"},{"line_number":211,"context_line":"        order_details[key] \u003d value"},{"line_number":212,"context_line":""},{"line_number":213,"context_line":"    try:"},{"line_number":214,"context_line":"        api.modify_order(**order_details)"}],"source_content_type":"text/x-python","patch_set":12,"id":"baada198_6608fb2e","line":211,"updated":"2014-08-07 18:03:48.000000000","message":"ditto above","commit_id":"33bbf3170890ec4065059381a3ba8c185cd72650"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"73874757202548b3d21193bd700cb7f4a14ca46e","unresolved":false,"context_lines":[{"line_number":130,"context_line":"    :returns: tuple with success, error message, and can retry"},{"line_number":131,"context_line":"    \"\"\""},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"    api \u003d Symantec(plugin_meta[\"username\"], plugin_meta[\"password\"],"},{"line_number":134,"context_line":"                   plugin_meta[\"url\"])"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"    order_details \u003d {}"}],"source_content_type":"text/x-python","patch_set":14,"id":"baada198_fd65511d","line":133,"updated":"2014-08-08 16:36:58.000000000","message":"The plugin_meta dict is for the plugin to store information in between method invocations from Barbican core. So the Symantec username, password and url information needs to come from config file options. See the top of dogtag.py for example.","commit_id":"88c598b19527a16396ad0e8157e0200462dcbf4a"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"73874757202548b3d21193bd700cb7f4a14ca46e","unresolved":false,"context_lines":[{"line_number":134,"context_line":"                   plugin_meta[\"url\"])"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"    order_details \u003d {}"},{"line_number":137,"context_line":"    order_details \u003d _populate(order_details, order_meta[\"OrderDetails\"])"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"    try:"},{"line_number":140,"context_line":"        order_data \u003d api.order(**order_details)"}],"source_content_type":"text/x-python","patch_set":14,"id":"baada198_803b0cc7","line":137,"updated":"2014-08-08 16:36:58.000000000","message":"Just pass use \u0027order_meta\u0027 directly in place of order_details. Details such as \u0027partnerCode\u0027 are name/values pairs stored in the order\u0027s \u0027meta\u0027 data field. That \u0027meta\u0027 section dict is passed into this plugin via the \u0027order_meta\u0027 dict. Therefore no need to pull off that dict from \u0027order_meta\u0027.  An no need for the _populate method.","commit_id":"88c598b19527a16396ad0e8157e0200462dcbf4a"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"73874757202548b3d21193bd700cb7f4a14ca46e","unresolved":false,"context_lines":[{"line_number":160,"context_line":"    plugin_meta parameters: partnerOrderId, partnerCode"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"    Additional parameters can be added to return additional details."},{"line_number":163,"context_line":"    These can be passed as a dict under order_meta[\"OrderStatus\"]"},{"line_number":164,"context_line":"    These additional options are found in Symantec\u0027s API docs."},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"    If the order is complete, the Certificate is also returned in the details."}],"source_content_type":"text/x-python","patch_set":14,"id":"baada198_20e2409c","line":163,"updated":"2014-08-08 16:36:58.000000000","message":"Additional Symantec specific parameters should be passed into this function outside of order_meta and plugin_meta.","commit_id":"88c598b19527a16396ad0e8157e0200462dcbf4a"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"73874757202548b3d21193bd700cb7f4a14ca46e","unresolved":false,"context_lines":[{"line_number":165,"context_line":""},{"line_number":166,"context_line":"    If the order is complete, the Certificate is also returned in the details."},{"line_number":167,"context_line":"    returns: tuple with success, error message, can retry,"},{"line_number":168,"context_line":"             and a dictionary of details."},{"line_number":169,"context_line":"    \"\"\""},{"line_number":170,"context_line":"    api \u003d Symantec(plugin_meta[\"username\"], plugin_meta[\"password\"],"},{"line_number":171,"context_line":"                   plugin_meta[\"url\"])"}],"source_content_type":"text/x-python","patch_set":14,"id":"baada198_406c9407","line":168,"updated":"2014-08-08 16:36:58.000000000","message":"Is the 4th entry in the response tuple needed here, or would the 3-tuple return of the other functions be ok here?","commit_id":"88c598b19527a16396ad0e8157e0200462dcbf4a"},{"author":{"_account_id":11661,"name":"Kevin Bishop","email":"ktb2012@att.net","username":"kbishop711"},"change_message_id":"b0197952923a7c771c7913aa2e47dcf14782e7a6","unresolved":false,"context_lines":[{"line_number":165,"context_line":""},{"line_number":166,"context_line":"    If the order is complete, the Certificate is also returned in the details."},{"line_number":167,"context_line":"    returns: tuple with success, error message, can retry,"},{"line_number":168,"context_line":"             and a dictionary of details."},{"line_number":169,"context_line":"    \"\"\""},{"line_number":170,"context_line":"    api \u003d Symantec(plugin_meta[\"username\"], plugin_meta[\"password\"],"},{"line_number":171,"context_line":"                   plugin_meta[\"url\"])"}],"source_content_type":"text/x-python","patch_set":14,"id":"baada198_49867fbb","line":168,"in_reply_to":"baada198_406c9407","updated":"2014-08-08 18:08:22.000000000","message":"Well, the SymantecSSL library parses the return xml from the api as a dict and returns that. If you just return that dict as the 4th entry, then you have all the information available to you: the order\u0027s status, the certificate if the status is complete, as well as other information if you requested more with the additional parameters (line 181).","commit_id":"88c598b19527a16396ad0e8157e0200462dcbf4a"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"73874757202548b3d21193bd700cb7f4a14ca46e","unresolved":false,"context_lines":[{"line_number":167,"context_line":"    returns: tuple with success, error message, can retry,"},{"line_number":168,"context_line":"             and a dictionary of details."},{"line_number":169,"context_line":"    \"\"\""},{"line_number":170,"context_line":"    api \u003d Symantec(plugin_meta[\"username\"], plugin_meta[\"password\"],"},{"line_number":171,"context_line":"                   plugin_meta[\"url\"])"},{"line_number":172,"context_line":""},{"line_number":173,"context_line":"    order_details \u003d {"}],"source_content_type":"text/x-python","patch_set":14,"id":"baada198_80a94cdf","line":170,"updated":"2014-08-08 16:36:58.000000000","message":"Same comments as above.","commit_id":"88c598b19527a16396ad0e8157e0200462dcbf4a"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"73874757202548b3d21193bd700cb7f4a14ca46e","unresolved":false,"context_lines":[{"line_number":178,"context_line":"        \"returncacerts\": \"TRUE\","},{"line_number":179,"context_line":"    }"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"    order_details \u003d _populate(order_details, order_meta[\"OrderStatus\"])"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"    try:"},{"line_number":184,"context_line":"        order_data \u003d api.get_order_by_partner_order_id(**order_details)"}],"source_content_type":"text/x-python","patch_set":14,"id":"baada198_80394c41","line":181,"updated":"2014-08-08 16:36:58.000000000","message":"This implies that the barbican core or the client must provide information to get the order status from Symantec. I\u0027d hope this isn\u0027t the case though, in which case this line isn\u0027t needed?","commit_id":"88c598b19527a16396ad0e8157e0200462dcbf4a"},{"author":{"_account_id":11661,"name":"Kevin Bishop","email":"ktb2012@att.net","username":"kbishop711"},"change_message_id":"b0197952923a7c771c7913aa2e47dcf14782e7a6","unresolved":false,"context_lines":[{"line_number":178,"context_line":"        \"returncacerts\": \"TRUE\","},{"line_number":179,"context_line":"    }"},{"line_number":180,"context_line":""},{"line_number":181,"context_line":"    order_details \u003d _populate(order_details, order_meta[\"OrderStatus\"])"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"    try:"},{"line_number":184,"context_line":"        order_data \u003d api.get_order_by_partner_order_id(**order_details)"}],"source_content_type":"text/x-python","patch_set":14,"id":"baada198_09672725","line":181,"in_reply_to":"baada198_80394c41","updated":"2014-08-08 18:08:22.000000000","message":"I intended that the information that is supposed to be added here would be additional parameters similar to \"returncertificateinfo\" above. I can separate that out from order_meta like you mentioned above, or just remove this if the only information we will ever request is the order status and the certificate if the order is complete.","commit_id":"88c598b19527a16396ad0e8157e0200462dcbf4a"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"73874757202548b3d21193bd700cb7f4a14ca46e","unresolved":false,"context_lines":[{"line_number":202,"context_line":""},{"line_number":203,"context_line":"    returns: tuple with success, error message, and can retry."},{"line_number":204,"context_line":"    \"\"\""},{"line_number":205,"context_line":"    api \u003d Symantec(plugin_meta[\"username\"], plugin_meta[\"password\"],"},{"line_number":206,"context_line":"                   plugin_meta[\"url\"])"},{"line_number":207,"context_line":""},{"line_number":208,"context_line":"    order_details \u003d {"}],"source_content_type":"text/x-python","patch_set":14,"id":"baada198_00ec5c28","line":205,"updated":"2014-08-08 16:36:58.000000000","message":"Same comments as above.","commit_id":"88c598b19527a16396ad0e8157e0200462dcbf4a"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"73874757202548b3d21193bd700cb7f4a14ca46e","unresolved":false,"context_lines":[{"line_number":211,"context_line":"        \"productCode\": plugin_meta[\"ProductCode\"],"},{"line_number":212,"context_line":"    }"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    order_details \u003d _populate(order_details, order_meta[\"ModifyDetails\"])"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":"    try:"},{"line_number":217,"context_line":"        api.modify_order(**order_details)"}],"source_content_type":"text/x-python","patch_set":14,"id":"baada198_a0bd70b4","line":214,"updated":"2014-08-08 16:36:58.000000000","message":"You should just be able to call order_details.update(order_meta)...no need for the _populate() method.","commit_id":"88c598b19527a16396ad0e8157e0200462dcbf4a"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"73874757202548b3d21193bd700cb7f4a14ca46e","unresolved":false,"context_lines":[{"line_number":233,"context_line":"    returns: tuple with success, error message, and can retry."},{"line_number":234,"context_line":"    \"\"\""},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"    plugin_meta[\"ModifyDetails\"] \u003d {\"ModifyOrderOperation\": \"CANCEL\"}"},{"line_number":237,"context_line":"    return _ca_modify_order(order_meta, plugin_meta)"}],"source_content_type":"text/x-python","patch_set":14,"id":"baada198_40e654e8","line":236,"updated":"2014-08-08 16:36:58.000000000","message":"Please don\u0027t use plugin_meta to store parameters to methods...this is intended as long-term persistence of plugin-specific data, such as the partnerOrderId stored above. I think it would be cleaner for the _ca_modify_order function to take a is_cancel_needed boolean, defaulting to False. Then the _ca_cancel_order() function can be removed.","commit_id":"88c598b19527a16396ad0e8157e0200462dcbf4a"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"e11183b90605ce586feec94e5c668ca1f1dc3d59","unresolved":false,"context_lines":[{"line_number":160,"context_line":"    api \u003d Symantec(self.username, self.password, self.url)"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"    try:"},{"line_number":163,"context_line":"        order_data \u003d api.order(**order_meta[\"OrderDetails\"])"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        # GeotrustOrderId is used to handle emails from Symantec."},{"line_number":166,"context_line":"        # PartnerCode and ProductCode are being stored in plugin_meta for"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_2f822bc7","line":163,"updated":"2014-08-08 19:36:25.000000000","message":"you shouldn\u0027t need \u0027OrderDetails\u0027 here...just **order_meta directly...it should have the name/values inside already.","commit_id":"657a3bc656bfe43113418ce849fefb83a153c50d"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"e11183b90605ce586feec94e5c668ca1f1dc3d59","unresolved":false,"context_lines":[{"line_number":189,"context_line":"    If the order is complete, the Certificate is also returned in the details."},{"line_number":190,"context_line":"    returns: tuple with success, error message, can retry,"},{"line_number":191,"context_line":"             and a dictionary of details."},{"line_number":192,"context_line":"    \"\"\""},{"line_number":193,"context_line":"    api \u003d Symantec(self.username, self.password, self.url)"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    order_details \u003d {"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_2f430b6c","line":192,"updated":"2014-08-08 19:36:25.000000000","message":"I think it would be better to just return status, error message and certificate data directly, so clients of this function don\u0027t have to know about the gory details of how Symantec returns things. If we need to return more info out of the response dict, that could be added later.","commit_id":"657a3bc656bfe43113418ce849fefb83a153c50d"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"e11183b90605ce586feec94e5c668ca1f1dc3d59","unresolved":false,"context_lines":[{"line_number":201,"context_line":"    }"},{"line_number":202,"context_line":""},{"line_number":203,"context_line":"    order_details.update(options)"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"    try:"},{"line_number":206,"context_line":"        order_data \u003d api.get_order_by_partner_order_id(**order_details)"},{"line_number":207,"context_line":"        return True, None, False, order_data"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_e1f927c6","line":204,"updated":"2014-08-08 19:36:25.000000000","message":"I think rather than an open dict of things to request called \u0027options\u0027, that has to be in a specific Symantec format, it would better to have specific boolean arguments added such as \u0027add_certificate_information\u0027. Within this function then, if \u0027add_certficate_information\u0027 was specified, then you would add to the order_details the specific Symantec name/value needed to get this extra info. Now the code outside of this function doesn\u0027t have to know about Symantec details as much.","commit_id":"657a3bc656bfe43113418ce849fefb83a153c50d"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"e11183b90605ce586feec94e5c668ca1f1dc3d59","unresolved":false,"context_lines":[{"line_number":233,"context_line":"    }"},{"line_number":234,"context_line":""},{"line_number":235,"context_line":"    if cancel:"},{"line_number":236,"context_line":"        modify_details[\"ModifyOrderOperation\"] \u003d \"CANCEL\""},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"    order_details.update(modify_details)"},{"line_number":239,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_e1be87cd","line":236,"updated":"2014-08-08 19:36:25.000000000","message":"This looks good. This is an example of what I\u0027m referring to in other comments then...in general on both inputs to Symantec\u0027s API, and in responses from it, it is cleaner to keep the Symantic name/value/xml details in these low level _ method as much as possible.","commit_id":"657a3bc656bfe43113418ce849fefb83a153c50d"},{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"ced5986a4ab2ac88253565b6612a365a4650aaef","unresolved":false,"context_lines":[{"line_number":152,"context_line":"    and tech contact info, and csr."},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"    Optional Parameters: techSameAsAdmin, billSameAsAdmin, more options can be"},{"line_number":155,"context_line":"    found in Symantec\u0027s API docs."},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"    :returns: tuple with success, error message, and can retry"},{"line_number":158,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":16,"id":"baada198_a4065d1f","line":155,"updated":"2014-08-08 20:38:48.000000000","message":"I hate to put links in code as they seem to be outdated as soon as you hit ENTER.\n\nThat said, might be a good idea to put a link in here to help the reader find these docs.","commit_id":"3a4621cc84ffcff48e3a75ef688c1f1e9c2eba31"},{"author":{"_account_id":9234,"name":"Steve Heyman","email":"steve@heyman.com","username":"hockeynut"},"change_message_id":"ced5986a4ab2ac88253565b6612a365a4650aaef","unresolved":false,"context_lines":[{"line_number":217,"context_line":""},{"line_number":218,"context_line":"    Also need a dict, modify_details  with modifyOrderOperation"},{"line_number":219,"context_line":"    and the parameters/values to modify. Modify options are found in Symantec\u0027s"},{"line_number":220,"context_line":"    API docs."},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"    returns: tuple with success, error message, and can retry."},{"line_number":223,"context_line":"    \"\"\""}],"source_content_type":"text/x-python","patch_set":16,"id":"baada198_c4ffd101","line":220,"updated":"2014-08-08 20:38:48.000000000","message":"same as above","commit_id":"3a4621cc84ffcff48e3a75ef688c1f1e9c2eba31"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"afca0c5c2d2d7626097586491910a784fb5d60a4","unresolved":false,"context_lines":[{"line_number":227,"context_line":"        \"productCode\": plugin_meta[\"ProductCode\"],"},{"line_number":228,"context_line":"    }"},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"    order_details.update(order_info)"},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"    try:"},{"line_number":233,"context_line":"        api.validate_order_parameters(**order_details)"}],"source_content_type":"text/x-python","patch_set":18,"id":"baada198_9a6c509f","line":230,"updated":"2014-08-08 21:11:21.000000000","message":"Can you change this to \u0027order_meta\u0027 for consistency?","commit_id":"67a8229a34af8d7dff62b6dce6e7ab422b3f0e3f"},{"author":{"_account_id":7874,"name":"Venkat Sundaram","email":"tsv@hp.com","username":"tsv"},"change_message_id":"5a4925c0ea451470e9201c5471fa3fd47642c7c7","unresolved":false,"context_lines":[{"line_number":93,"context_line":"                            this plugin. Plugins may also update/add"},{"line_number":94,"context_line":"                            information here which Barbican will persist"},{"line_number":95,"context_line":"                            on their behalf."},{"line_number":96,"context_line":"        \"\"\""},{"line_number":97,"context_line":"        raise NotImplementedError  # pragma: no cover"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"    def cancel_certificate_request(self, order_id, order_meta, plugin_meta):"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_0b28eaec","line":96,"updated":"2014-08-09 22:14:05.000000000","message":"Why not implemented ? Shouldn\u0027t this be wired up with the corresponding private method ?","commit_id":"c3b51cfaa81ce1bb9ff33a96232e381ab9b43268"},{"author":{"_account_id":7262,"name":"John Vrbanac","email":"john.vrbanac@linux.com","username":"john.vrbanac"},"change_message_id":"e03c9a0e2a2f59cc036f9996739a600003054258","unresolved":false,"context_lines":[{"line_number":93,"context_line":"                            this plugin. Plugins may also update/add"},{"line_number":94,"context_line":"                            information here which Barbican will persist"},{"line_number":95,"context_line":"                            on their behalf."},{"line_number":96,"context_line":"        \"\"\""},{"line_number":97,"context_line":"        raise NotImplementedError  # pragma: no cover"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"    def cancel_certificate_request(self, order_id, order_meta, plugin_meta):"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_ad7b72bd","line":96,"in_reply_to":"baada198_0b28eaec","updated":"2014-08-11 15:34:14.000000000","message":"These are not in lines changed or are relevant to a changed block of code.","commit_id":"c3b51cfaa81ce1bb9ff33a96232e381ab9b43268"},{"author":{"_account_id":7262,"name":"John Vrbanac","email":"john.vrbanac@linux.com","username":"john.vrbanac"},"change_message_id":"42f4f2387608da7329dca0b6f4eb4ad5d4875151","unresolved":false,"context_lines":[{"line_number":93,"context_line":"                            this plugin. Plugins may also update/add"},{"line_number":94,"context_line":"                            information here which Barbican will persist"},{"line_number":95,"context_line":"                            on their behalf."},{"line_number":96,"context_line":"        \"\"\""},{"line_number":97,"context_line":"        raise NotImplementedError  # pragma: no cover"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"    def cancel_certificate_request(self, order_id, order_meta, plugin_meta):"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_488f1d76","line":96,"in_reply_to":"baada198_1c2e4a42","updated":"2014-08-11 18:31:54.000000000","message":"The commit message doesn\u0027t say that it\u0027s implementing a fully  functioning integration of the library. It\u0027s just a first pass at starting the integration. I would expect for these methods to be enabled when the integration is complete and in a future CR.","commit_id":"c3b51cfaa81ce1bb9ff33a96232e381ab9b43268"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"811632919a1560749076296de11a1697dcc99973","unresolved":false,"context_lines":[{"line_number":93,"context_line":"                            this plugin. Plugins may also update/add"},{"line_number":94,"context_line":"                            information here which Barbican will persist"},{"line_number":95,"context_line":"                            on their behalf."},{"line_number":96,"context_line":"        \"\"\""},{"line_number":97,"context_line":"        raise NotImplementedError  # pragma: no cover"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"    def cancel_certificate_request(self, order_id, order_meta, plugin_meta):"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_52b7a6d6","line":96,"in_reply_to":"baada198_488f1d76","updated":"2014-08-12 13:54:00.000000000","message":"That\u0027s correct. The idea was to get a smaller CR in place to star the ball roling on this Symantec work, and to also provide other CA plugin developers an example to follow.","commit_id":"c3b51cfaa81ce1bb9ff33a96232e381ab9b43268"},{"author":{"_account_id":7874,"name":"Venkat Sundaram","email":"tsv@hp.com","username":"tsv"},"change_message_id":"62a4564cd60cfe1a1c767daf360efb867003eca5","unresolved":false,"context_lines":[{"line_number":93,"context_line":"                            this plugin. Plugins may also update/add"},{"line_number":94,"context_line":"                            information here which Barbican will persist"},{"line_number":95,"context_line":"                            on their behalf."},{"line_number":96,"context_line":"        \"\"\""},{"line_number":97,"context_line":"        raise NotImplementedError  # pragma: no cover"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"    def cancel_certificate_request(self, order_id, order_meta, plugin_meta):"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_1c2e4a42","line":96,"in_reply_to":"baada198_ad7b72bd","updated":"2014-08-11 17:34:27.000000000","message":"The issue_certificate_request method implemented above calls the private method _ca_create_order. Similar to that, there is a private method _ca_modify_order. If that method is not called from here, how will that be called ? Or, is that expected to be done in a future commit ? Could you please explain why it is not relevant ?","commit_id":"c3b51cfaa81ce1bb9ff33a96232e381ab9b43268"},{"author":{"_account_id":7874,"name":"Venkat Sundaram","email":"tsv@hp.com","username":"tsv"},"change_message_id":"5a4925c0ea451470e9201c5471fa3fd47642c7c7","unresolved":false,"context_lines":[{"line_number":105,"context_line":"                            this plugin. Plugins may also update/add"},{"line_number":106,"context_line":"                            information here which Barbican will persist"},{"line_number":107,"context_line":"                            on their behalf."},{"line_number":108,"context_line":"        \"\"\""},{"line_number":109,"context_line":"        raise NotImplementedError  # pragma: no cover"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    def check_certificate_status(self, order_id, order_meta, plugin_meta):"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_2b2daefd","line":108,"updated":"2014-08-09 22:14:05.000000000","message":"same as above","commit_id":"c3b51cfaa81ce1bb9ff33a96232e381ab9b43268"},{"author":{"_account_id":7262,"name":"John Vrbanac","email":"john.vrbanac@linux.com","username":"john.vrbanac"},"change_message_id":"e03c9a0e2a2f59cc036f9996739a600003054258","unresolved":false,"context_lines":[{"line_number":105,"context_line":"                            this plugin. Plugins may also update/add"},{"line_number":106,"context_line":"                            information here which Barbican will persist"},{"line_number":107,"context_line":"                            on their behalf."},{"line_number":108,"context_line":"        \"\"\""},{"line_number":109,"context_line":"        raise NotImplementedError  # pragma: no cover"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    def check_certificate_status(self, order_id, order_meta, plugin_meta):"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_cd76e6d4","line":108,"in_reply_to":"baada198_2b2daefd","updated":"2014-08-11 15:34:14.000000000","message":"These are not in lines changed or are relevant to a changed block of code.","commit_id":"c3b51cfaa81ce1bb9ff33a96232e381ab9b43268"},{"author":{"_account_id":7262,"name":"John Vrbanac","email":"john.vrbanac@linux.com","username":"john.vrbanac"},"change_message_id":"42f4f2387608da7329dca0b6f4eb4ad5d4875151","unresolved":false,"context_lines":[{"line_number":105,"context_line":"                            this plugin. Plugins may also update/add"},{"line_number":106,"context_line":"                            information here which Barbican will persist"},{"line_number":107,"context_line":"                            on their behalf."},{"line_number":108,"context_line":"        \"\"\""},{"line_number":109,"context_line":"        raise NotImplementedError  # pragma: no cover"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    def check_certificate_status(self, order_id, order_meta, plugin_meta):"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_688a2166","line":108,"in_reply_to":"baada198_9c197a25","updated":"2014-08-11 18:31:54.000000000","message":"see above","commit_id":"c3b51cfaa81ce1bb9ff33a96232e381ab9b43268"},{"author":{"_account_id":7874,"name":"Venkat Sundaram","email":"tsv@hp.com","username":"tsv"},"change_message_id":"62a4564cd60cfe1a1c767daf360efb867003eca5","unresolved":false,"context_lines":[{"line_number":105,"context_line":"                            this plugin. Plugins may also update/add"},{"line_number":106,"context_line":"                            information here which Barbican will persist"},{"line_number":107,"context_line":"                            on their behalf."},{"line_number":108,"context_line":"        \"\"\""},{"line_number":109,"context_line":"        raise NotImplementedError  # pragma: no cover"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"    def check_certificate_status(self, order_id, order_meta, plugin_meta):"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_9c197a25","line":108,"in_reply_to":"baada198_cd76e6d4","updated":"2014-08-11 17:34:27.000000000","message":"The issue_certificate_request method implemented above calls the private method _ca_create_order. Similar to that, there is a private method _ca_cancel_order. If that method is not called from here, how will that be called ? Or, is that expected to be done in a future commit ? Could you please explain why it is not relevant ?","commit_id":"c3b51cfaa81ce1bb9ff33a96232e381ab9b43268"},{"author":{"_account_id":7874,"name":"Venkat Sundaram","email":"tsv@hp.com","username":"tsv"},"change_message_id":"5a4925c0ea451470e9201c5471fa3fd47642c7c7","unresolved":false,"context_lines":[{"line_number":117,"context_line":"                            this plugin. Plugins may also update/add"},{"line_number":118,"context_line":"                            information here which Barbican will persist"},{"line_number":119,"context_line":"                            on their behalf."},{"line_number":120,"context_line":"        \"\"\""},{"line_number":121,"context_line":"        raise NotImplementedError  # pragma: no cover"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"    def supports(self, certificate_spec):"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_4b2e62fa","line":120,"updated":"2014-08-09 22:14:05.000000000","message":"same as above","commit_id":"c3b51cfaa81ce1bb9ff33a96232e381ab9b43268"},{"author":{"_account_id":7874,"name":"Venkat Sundaram","email":"tsv@hp.com","username":"tsv"},"change_message_id":"62a4564cd60cfe1a1c767daf360efb867003eca5","unresolved":false,"context_lines":[{"line_number":117,"context_line":"                            this plugin. Plugins may also update/add"},{"line_number":118,"context_line":"                            information here which Barbican will persist"},{"line_number":119,"context_line":"                            on their behalf."},{"line_number":120,"context_line":"        \"\"\""},{"line_number":121,"context_line":"        raise NotImplementedError  # pragma: no cover"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"    def supports(self, certificate_spec):"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_dc0f72d2","line":120,"in_reply_to":"baada198_2d3f62e0","updated":"2014-08-11 17:34:27.000000000","message":"The issue_certificate_request method implemented above calls the private method _ca_create_order. Similar to that, there is a private method _ca_get_order_status. If that method is not called from here, how will that be called ? Or, is that expected to be done in a future commit ? Could you please explain why it is not relevant ?","commit_id":"c3b51cfaa81ce1bb9ff33a96232e381ab9b43268"},{"author":{"_account_id":7262,"name":"John Vrbanac","email":"john.vrbanac@linux.com","username":"john.vrbanac"},"change_message_id":"e03c9a0e2a2f59cc036f9996739a600003054258","unresolved":false,"context_lines":[{"line_number":117,"context_line":"                            this plugin. Plugins may also update/add"},{"line_number":118,"context_line":"                            information here which Barbican will persist"},{"line_number":119,"context_line":"                            on their behalf."},{"line_number":120,"context_line":"        \"\"\""},{"line_number":121,"context_line":"        raise NotImplementedError  # pragma: no cover"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"    def supports(self, certificate_spec):"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_2d3f62e0","line":120,"in_reply_to":"baada198_4b2e62fa","updated":"2014-08-11 15:34:14.000000000","message":"These are not in lines changed or are relevant to a changed block of code.","commit_id":"c3b51cfaa81ce1bb9ff33a96232e381ab9b43268"},{"author":{"_account_id":7262,"name":"John Vrbanac","email":"john.vrbanac@linux.com","username":"john.vrbanac"},"change_message_id":"42f4f2387608da7329dca0b6f4eb4ad5d4875151","unresolved":false,"context_lines":[{"line_number":117,"context_line":"                            this plugin. Plugins may also update/add"},{"line_number":118,"context_line":"                            information here which Barbican will persist"},{"line_number":119,"context_line":"                            on their behalf."},{"line_number":120,"context_line":"        \"\"\""},{"line_number":121,"context_line":"        raise NotImplementedError  # pragma: no cover"},{"line_number":122,"context_line":""},{"line_number":123,"context_line":"    def supports(self, certificate_spec):"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_8885b554","line":120,"in_reply_to":"baada198_dc0f72d2","updated":"2014-08-11 18:31:54.000000000","message":"see above","commit_id":"c3b51cfaa81ce1bb9ff33a96232e381ab9b43268"},{"author":{"_account_id":7874,"name":"Venkat Sundaram","email":"tsv@hp.com","username":"tsv"},"change_message_id":"62a4564cd60cfe1a1c767daf360efb867003eca5","unresolved":false,"context_lines":[{"line_number":211,"context_line":"def _ca_modify_order(self, order_meta, plugin_meta):"},{"line_number":212,"context_line":"    \"\"\"Sends a request to the Symantec CA to modify an order."},{"line_number":213,"context_line":"    Parameters needed for modifyOrder:"},{"line_number":214,"context_line":"        partnerorderid - Needed to specify order"},{"line_number":215,"context_line":"        partnercode - Needed to specify order"},{"line_number":216,"context_line":"        productcode - Needed to specify order"},{"line_number":217,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_e203b009","line":214,"updated":"2014-08-11 17:34:27.000000000","message":"nit: please change these parameter names to match with the case of actual variable names","commit_id":"c3b51cfaa81ce1bb9ff33a96232e381ab9b43268"},{"author":{"_account_id":7874,"name":"Venkat Sundaram","email":"tsv@hp.com","username":"tsv"},"change_message_id":"5a4925c0ea451470e9201c5471fa3fd47642c7c7","unresolved":false,"context_lines":[{"line_number":230,"context_line":"    order_details.update(order_meta)"},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"    try:"},{"line_number":233,"context_line":"        api.validate_order_parameters(**order_details)"},{"line_number":234,"context_line":"        return True, None, False"},{"line_number":235,"context_line":"    except symantec_exceptions.SymantecError as e:"},{"line_number":236,"context_line":"        return False, e, False"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_abf1be97","line":233,"updated":"2014-08-09 22:14:05.000000000","message":"Shouldn\u0027t there be a call to modify after the validation ?:\n\n api.modify_order(**order_details)","commit_id":"c3b51cfaa81ce1bb9ff33a96232e381ab9b43268"},{"author":{"_account_id":7874,"name":"Venkat Sundaram","email":"tsv@hp.com","username":"tsv"},"change_message_id":"62a4564cd60cfe1a1c767daf360efb867003eca5","unresolved":false,"context_lines":[{"line_number":230,"context_line":"    order_details.update(order_meta)"},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"    try:"},{"line_number":233,"context_line":"        api.validate_order_parameters(**order_details)"},{"line_number":234,"context_line":"        return True, None, False"},{"line_number":235,"context_line":"    except symantec_exceptions.SymantecError as e:"},{"line_number":236,"context_line":"        return False, e, False"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_dc3e9225","line":233,"in_reply_to":"baada198_81dbd57b","updated":"2014-08-11 17:34:27.000000000","message":"Thanks for clarifying, though like you said, unfortunate naming conventions(modify method calls validate_order and the cancel method calls modify_order). Perhaps a comment before the \"api.validate_order_parameters(**order_details)\" call would help ?","commit_id":"c3b51cfaa81ce1bb9ff33a96232e381ab9b43268"},{"author":{"_account_id":11661,"name":"Kevin Bishop","email":"ktb2012@att.net","username":"kbishop711"},"change_message_id":"3f556755773f0892bad5a45fc14dff7741c0f4de","unresolved":false,"context_lines":[{"line_number":230,"context_line":"    order_details.update(order_meta)"},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"    try:"},{"line_number":233,"context_line":"        api.validate_order_parameters(**order_details)"},{"line_number":234,"context_line":"        return True, None, False"},{"line_number":235,"context_line":"    except symantec_exceptions.SymantecError as e:"},{"line_number":236,"context_line":"        return False, e, False"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_81dbd57b","line":233,"in_reply_to":"baada198_abf1be97","updated":"2014-08-11 16:40:38.000000000","message":"This is intentional. \"validate_order_parameters\" is the name of the Symantec API method for changing details of a client\u0027s order. If no values are passed in, then the order\u0027s details are just returned by the API. \"modify_order\" is for changing the status of an order, so that method is for cancelling, approving, etc.","commit_id":"c3b51cfaa81ce1bb9ff33a96232e381ab9b43268"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"811632919a1560749076296de11a1697dcc99973","unresolved":false,"context_lines":[{"line_number":230,"context_line":"    order_details.update(order_meta)"},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"    try:"},{"line_number":233,"context_line":"        api.validate_order_parameters(**order_details)"},{"line_number":234,"context_line":"        return True, None, False"},{"line_number":235,"context_line":"    except symantec_exceptions.SymantecError as e:"},{"line_number":236,"context_line":"        return False, e, False"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_92fbfe78","line":233,"in_reply_to":"baada198_dc3e9225","updated":"2014-08-12 13:54:00.000000000","message":"Agreed.","commit_id":"c3b51cfaa81ce1bb9ff33a96232e381ab9b43268"},{"author":{"_account_id":7874,"name":"Venkat Sundaram","email":"tsv@hp.com","username":"tsv"},"change_message_id":"5a4925c0ea451470e9201c5471fa3fd47642c7c7","unresolved":false,"context_lines":[{"line_number":240,"context_line":""},{"line_number":241,"context_line":"def _ca_cancel_order(self, plugin_meta):"},{"line_number":242,"context_line":"    \"\"\"Sends a request to the Symantec CA to cancel an order."},{"line_number":243,"context_line":"    Parameters needed for modifyOrder:"},{"line_number":244,"context_line":"        partnerorderid - Needed to specify order"},{"line_number":245,"context_line":"        partnercode - Needed to specify order"},{"line_number":246,"context_line":"        productcode - Needed to specify order"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_ebd016f6","line":243,"updated":"2014-08-09 22:14:05.000000000","message":"Shouldn\u0027t this be cancelOrder ?","commit_id":"c3b51cfaa81ce1bb9ff33a96232e381ab9b43268"},{"author":{"_account_id":7874,"name":"Venkat Sundaram","email":"tsv@hp.com","username":"tsv"},"change_message_id":"62a4564cd60cfe1a1c767daf360efb867003eca5","unresolved":false,"context_lines":[{"line_number":240,"context_line":""},{"line_number":241,"context_line":"def _ca_cancel_order(self, plugin_meta):"},{"line_number":242,"context_line":"    \"\"\"Sends a request to the Symantec CA to cancel an order."},{"line_number":243,"context_line":"    Parameters needed for modifyOrder:"},{"line_number":244,"context_line":"        partnerorderid - Needed to specify order"},{"line_number":245,"context_line":"        partnercode - Needed to specify order"},{"line_number":246,"context_line":"        productcode - Needed to specify order"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_3c078e71","line":243,"in_reply_to":"baada198_41ee1d22","updated":"2014-08-11 17:34:27.000000000","message":"Got it now. Thanks.","commit_id":"c3b51cfaa81ce1bb9ff33a96232e381ab9b43268"},{"author":{"_account_id":11661,"name":"Kevin Bishop","email":"ktb2012@att.net","username":"kbishop711"},"change_message_id":"3f556755773f0892bad5a45fc14dff7741c0f4de","unresolved":false,"context_lines":[{"line_number":240,"context_line":""},{"line_number":241,"context_line":"def _ca_cancel_order(self, plugin_meta):"},{"line_number":242,"context_line":"    \"\"\"Sends a request to the Symantec CA to cancel an order."},{"line_number":243,"context_line":"    Parameters needed for modifyOrder:"},{"line_number":244,"context_line":"        partnerorderid - Needed to specify order"},{"line_number":245,"context_line":"        partnercode - Needed to specify order"},{"line_number":246,"context_line":"        productcode - Needed to specify order"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_41ee1d22","line":243,"in_reply_to":"baada198_ebd016f6","updated":"2014-08-11 16:40:38.000000000","message":"See above","commit_id":"c3b51cfaa81ce1bb9ff33a96232e381ab9b43268"},{"author":{"_account_id":7874,"name":"Venkat Sundaram","email":"tsv@hp.com","username":"tsv"},"change_message_id":"62a4564cd60cfe1a1c767daf360efb867003eca5","unresolved":false,"context_lines":[{"line_number":241,"context_line":"def _ca_cancel_order(self, plugin_meta):"},{"line_number":242,"context_line":"    \"\"\"Sends a request to the Symantec CA to cancel an order."},{"line_number":243,"context_line":"    Parameters needed for modifyOrder:"},{"line_number":244,"context_line":"        partnerorderid - Needed to specify order"},{"line_number":245,"context_line":"        partnercode - Needed to specify order"},{"line_number":246,"context_line":"        productcode - Needed to specify order"},{"line_number":247,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_dcfbf213","line":244,"updated":"2014-08-11 17:34:27.000000000","message":"nit: please change these parameter names to match with the case of actual variable names","commit_id":"c3b51cfaa81ce1bb9ff33a96232e381ab9b43268"},{"author":{"_account_id":7262,"name":"John Vrbanac","email":"john.vrbanac@linux.com","username":"john.vrbanac"},"change_message_id":"ff4683482e0662c64751a0bae67c4911b60a97d4","unresolved":false,"context_lines":[{"line_number":241,"context_line":"def _ca_cancel_order(self, plugin_meta):"},{"line_number":242,"context_line":"    \"\"\"Sends a request to the Symantec CA to cancel an order."},{"line_number":243,"context_line":"    Parameters needed for modifyOrder:"},{"line_number":244,"context_line":"        partnerorderid - Needed to specify order"},{"line_number":245,"context_line":"        partnercode - Needed to specify order"},{"line_number":246,"context_line":"        productcode - Needed to specify order"},{"line_number":247,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"9ab29df4_6b44a974","line":244,"in_reply_to":"baada198_b26ac218","updated":"2014-08-13 20:36:54.000000000","message":"I\u0027ve updated these docstrings in: https://review.openstack.org/#/c/114017/","commit_id":"c3b51cfaa81ce1bb9ff33a96232e381ab9b43268"},{"author":{"_account_id":7789,"name":"John Wood","email":"john.wood@rackspace.com","username":"woodster"},"change_message_id":"811632919a1560749076296de11a1697dcc99973","unresolved":false,"context_lines":[{"line_number":241,"context_line":"def _ca_cancel_order(self, plugin_meta):"},{"line_number":242,"context_line":"    \"\"\"Sends a request to the Symantec CA to cancel an order."},{"line_number":243,"context_line":"    Parameters needed for modifyOrder:"},{"line_number":244,"context_line":"        partnerorderid - Needed to specify order"},{"line_number":245,"context_line":"        partnercode - Needed to specify order"},{"line_number":246,"context_line":"        productcode - Needed to specify order"},{"line_number":247,"context_line":""}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_b26ac218","line":244,"in_reply_to":"baada198_dcfbf213","updated":"2014-08-12 13:54:00.000000000","message":"Oh, to sync up with actual key names used, such as at line #255 (and other lines) . That makes sense.","commit_id":"c3b51cfaa81ce1bb9ff33a96232e381ab9b43268"},{"author":{"_account_id":7874,"name":"Venkat Sundaram","email":"tsv@hp.com","username":"tsv"},"change_message_id":"5a4925c0ea451470e9201c5471fa3fd47642c7c7","unresolved":false,"context_lines":[{"line_number":247,"context_line":""},{"line_number":248,"context_line":"    returns: tuple with success, error message, and can retry."},{"line_number":249,"context_line":"    \"\"\""},{"line_number":250,"context_line":"    api \u003d Symantec(self.username, self.password, self.url)"},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"    order_details \u003d {"},{"line_number":253,"context_line":"        \"partnerOrderId\": plugin_meta[\"PartnerOrderID\"],"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_6bfc0672","line":250,"updated":"2014-08-09 22:14:05.000000000","message":"Instead of repeating the same modify order code here, why not do something like below ?\n\norder_details \u003d { \"modifyOrderOperation\": \"CANCEL\" }\nreturn self._ca_modify_order(order_details, plugin_meta)\n\nNoticed that Steve left similar comment for an earlier patch set. Any reason why that is not implemented ?","commit_id":"c3b51cfaa81ce1bb9ff33a96232e381ab9b43268"},{"author":{"_account_id":7874,"name":"Venkat Sundaram","email":"tsv@hp.com","username":"tsv"},"change_message_id":"62a4564cd60cfe1a1c767daf360efb867003eca5","unresolved":false,"context_lines":[{"line_number":247,"context_line":""},{"line_number":248,"context_line":"    returns: tuple with success, error message, and can retry."},{"line_number":249,"context_line":"    \"\"\""},{"line_number":250,"context_line":"    api \u003d Symantec(self.username, self.password, self.url)"},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"    order_details \u003d {"},{"line_number":253,"context_line":"        \"partnerOrderId\": plugin_meta[\"PartnerOrderID\"],"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_fc2fd6a4","line":250,"in_reply_to":"baada198_21ebe912","updated":"2014-08-11 17:34:27.000000000","message":"Got it. Thanks","commit_id":"c3b51cfaa81ce1bb9ff33a96232e381ab9b43268"},{"author":{"_account_id":11661,"name":"Kevin Bishop","email":"ktb2012@att.net","username":"kbishop711"},"change_message_id":"3f556755773f0892bad5a45fc14dff7741c0f4de","unresolved":false,"context_lines":[{"line_number":247,"context_line":""},{"line_number":248,"context_line":"    returns: tuple with success, error message, and can retry."},{"line_number":249,"context_line":"    \"\"\""},{"line_number":250,"context_line":"    api \u003d Symantec(self.username, self.password, self.url)"},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"    order_details \u003d {"},{"line_number":253,"context_line":"        \"partnerOrderId\": plugin_meta[\"PartnerOrderID\"],"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_21ebe912","line":250,"in_reply_to":"baada198_6bfc0672","updated":"2014-08-11 16:40:38.000000000","message":"Unfortunately, you can\u0027t do that since _ca_modify_order and _ca_cancel_order use different API methods.","commit_id":"c3b51cfaa81ce1bb9ff33a96232e381ab9b43268"}]}