)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"3e64fa31b932b72319241d1091ab832a72b306cc","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"I haven\u0027t included a unit test for the new method because"},{"line_number":20,"context_line":"ServiceStatusPayload uses the new method and that notification is"},{"line_number":21,"context_line":"tested, so we already have coverage."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Change-Id: Ia4fb4dd004f0f0fe0006852e2411ddcb2bb84fd4"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"1a122d0e_cb336261","line":21,"updated":"2016-05-03 13:08:56.000000000","message":"I would still add a generic unit test so that if the ServiceStatusPayload is changed or removed we still have coverage on the new generic function.","commit_id":"ab76f6eb4e9593a91b3abb0e89489cbe62bafa78"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"695f8fe44037d15ec3959a7f213053c18b8dd287","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"I haven\u0027t included a unit test for the new method because"},{"line_number":20,"context_line":"ServiceStatusPayload uses the new method and that notification is"},{"line_number":21,"context_line":"tested, so we already have coverage."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Change-Id: Ia4fb4dd004f0f0fe0006852e2411ddcb2bb84fd4"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"1a122d0e_81802b4b","line":21,"in_reply_to":"1a122d0e_cb336261","updated":"2016-05-03 13:40:03.000000000","message":"Done","commit_id":"ab76f6eb4e9593a91b3abb0e89489cbe62bafa78"}],"nova/objects/notification.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"3e64fa31b932b72319241d1091ab832a72b306cc","unresolved":false,"context_lines":[{"line_number":68,"context_line":"        super(NotificationPayloadBase, self).__init__(*args, **kwargs)"},{"line_number":69,"context_line":"        self.populated \u003d not self.SCHEMA"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"    def mk_simple_schema(self, src\u003d\u0027src\u0027):"},{"line_number":72,"context_line":"        \"\"\"Make a simple schema assuming a single data source."},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        The SCHEMA may be further customised after calling this helper."}],"source_content_type":"text/x-python","patch_set":1,"id":"1a122d0e_2b4ad6ac","line":71,"range":{"start_line":71,"start_character":8,"end_line":71,"end_character":24},"updated":"2016-05-03 13:08:56.000000000","message":"Does define_schema_from_fields(self, source_name) a better name?","commit_id":"ab76f6eb4e9593a91b3abb0e89489cbe62bafa78"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"695f8fe44037d15ec3959a7f213053c18b8dd287","unresolved":false,"context_lines":[{"line_number":68,"context_line":"        super(NotificationPayloadBase, self).__init__(*args, **kwargs)"},{"line_number":69,"context_line":"        self.populated \u003d not self.SCHEMA"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"    def mk_simple_schema(self, src\u003d\u0027src\u0027):"},{"line_number":72,"context_line":"        \"\"\"Make a simple schema assuming a single data source."},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        The SCHEMA may be further customised after calling this helper."}],"source_content_type":"text/x-python","patch_set":1,"id":"1a122d0e_01953b87","line":71,"range":{"start_line":71,"start_character":8,"end_line":71,"end_character":24},"in_reply_to":"1a122d0e_2b4ad6ac","updated":"2016-05-03 13:40:03.000000000","message":"No; but I\u0027ve tried to be more descriptive","commit_id":"ab76f6eb4e9593a91b3abb0e89489cbe62bafa78"}],"nova/objects/service.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"3e64fa31b932b72319241d1091ab832a72b306cc","unresolved":false,"context_lines":[{"line_number":457,"context_line":"        \u0027last_seen_up\u0027: (\u0027service\u0027, \u0027last_seen_up\u0027),"},{"line_number":458,"context_line":"        \u0027forced_down\u0027: (\u0027service\u0027, \u0027forced_down\u0027),"},{"line_number":459,"context_line":"        \u0027version\u0027: (\u0027service\u0027, \u0027version\u0027)"},{"line_number":460,"context_line":"    }"},{"line_number":461,"context_line":"    # Version 1.0: Initial version"},{"line_number":462,"context_line":"    VERSION \u003d \u00271.0\u0027"},{"line_number":463,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a122d0e_ebabbe1c","side":"PARENT","line":460,"updated":"2016-05-03 13:08:56.000000000","message":"I think this was a lot more explicit and I think this really helps defining the semantic of the notification API.","commit_id":"82098d06dbaf401966a70c873b8aa97e7eab4b10"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"3d1b4b862c06b720b9e0a215aa18d9cd0c10bd16","unresolved":false,"context_lines":[{"line_number":457,"context_line":"        \u0027last_seen_up\u0027: (\u0027service\u0027, \u0027last_seen_up\u0027),"},{"line_number":458,"context_line":"        \u0027forced_down\u0027: (\u0027service\u0027, \u0027forced_down\u0027),"},{"line_number":459,"context_line":"        \u0027version\u0027: (\u0027service\u0027, \u0027version\u0027)"},{"line_number":460,"context_line":"    }"},{"line_number":461,"context_line":"    # Version 1.0: Initial version"},{"line_number":462,"context_line":"    VERSION \u003d \u00271.0\u0027"},{"line_number":463,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a122d0e_013edb28","side":"PARENT","line":460,"in_reply_to":"1a122d0e_01dc1b73","updated":"2016-05-03 14:07:59.000000000","message":"I meant the semantics of the payload of the service-update notification. The call of mk_simple_schema() does not really define for me that the content of the fields will be copied from the service object. See my suggestion in the other comment.","commit_id":"82098d06dbaf401966a70c873b8aa97e7eab4b10"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"695f8fe44037d15ec3959a7f213053c18b8dd287","unresolved":false,"context_lines":[{"line_number":457,"context_line":"        \u0027last_seen_up\u0027: (\u0027service\u0027, \u0027last_seen_up\u0027),"},{"line_number":458,"context_line":"        \u0027forced_down\u0027: (\u0027service\u0027, \u0027forced_down\u0027),"},{"line_number":459,"context_line":"        \u0027version\u0027: (\u0027service\u0027, \u0027version\u0027)"},{"line_number":460,"context_line":"    }"},{"line_number":461,"context_line":"    # Version 1.0: Initial version"},{"line_number":462,"context_line":"    VERSION \u003d \u00271.0\u0027"},{"line_number":463,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":1,"id":"1a122d0e_01dc1b73","side":"PARENT","line":460,"in_reply_to":"1a122d0e_ebabbe1c","updated":"2016-05-03 13:40:03.000000000","message":"The API should be defined at the definition, not the usage site. IE in nova/objects/notification.py. I\u0027ve added a large comment (incl. example) to my new method; there is already a massive one on the NotificationPayloadBase class but I guess you feel that is insufficient.","commit_id":"82098d06dbaf401966a70c873b8aa97e7eab4b10"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"51f6dc0dda5eb0087893070edcd97c152f85ae75","unresolved":false,"context_lines":[{"line_number":463,"context_line":""},{"line_number":464,"context_line":"    def __init__(self, service):"},{"line_number":465,"context_line":"        super(ServiceStatusPayload, self).__init__()"},{"line_number":466,"context_line":"        self.single_source_schema()"},{"line_number":467,"context_line":"        self.populate_schema(src\u003dservice)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a122d0e_3cf17a96","line":467,"range":{"start_line":466,"start_character":0,"end_line":467,"end_character":41},"updated":"2016-05-03 14:09:23.000000000","message":"I think this the default behavior \u0027fill all fields from a single object provided\u0027 can be made more readable with something like:\n    self.populate_fields(src\u003dservice)\n\nand then in the PayloadBase the populate_fields(src) can iterate through the self.fields and copy the value from the same field of the src object.\n\nWhat do you think?","commit_id":"0e096dec15fad7a046b3b1893b91cfbf6e76ce33"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"dc5e8c3034b3d130c9e5d2dc7250c447ba3577a1","unresolved":false,"context_lines":[{"line_number":463,"context_line":""},{"line_number":464,"context_line":"    def __init__(self, service):"},{"line_number":465,"context_line":"        super(ServiceStatusPayload, self).__init__()"},{"line_number":466,"context_line":"        self.single_source_schema()"},{"line_number":467,"context_line":"        self.populate_schema(src\u003dservice)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a122d0e_8130fd08","line":467,"range":{"start_line":466,"start_character":0,"end_line":467,"end_character":41},"in_reply_to":"1a122d0e_28d62ef1","updated":"2016-05-04 13:30:08.000000000","message":"Thanks for taking the time to review :)","commit_id":"0e096dec15fad7a046b3b1893b91cfbf6e76ce33"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"d629b53481034fec8d42b860fcef418709d4cb9a","unresolved":false,"context_lines":[{"line_number":463,"context_line":""},{"line_number":464,"context_line":"    def __init__(self, service):"},{"line_number":465,"context_line":"        super(ServiceStatusPayload, self).__init__()"},{"line_number":466,"context_line":"        self.single_source_schema()"},{"line_number":467,"context_line":"        self.populate_schema(src\u003dservice)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a122d0e_d791419c","line":467,"range":{"start_line":466,"start_character":0,"end_line":467,"end_character":41},"in_reply_to":"1a122d0e_3cf17a96","updated":"2016-05-03 14:17:49.000000000","message":"That removes the ability to customise the schema in between. For example, almost all the fields might match, but there is one oddball. As I have it, you can do:\n\n  self.single_source_schema()\n  SCHEMA[\u0027host\u0027] \u003d (\u0027src\u0027, \u0027dest_host\u0027)\n  self.populate_schema(src\u003dservice)\n\nWhat we could do is add a convenience method like \"populate_single_source_schema\" that calls both methods in succession. It\u0027d be so small though that I\u0027m not sure it\u0027s worth it.","commit_id":"0e096dec15fad7a046b3b1893b91cfbf6e76ce33"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"b27a451bdf81a8e06b2e2698f9aa6b93daaf6390","unresolved":false,"context_lines":[{"line_number":463,"context_line":""},{"line_number":464,"context_line":"    def __init__(self, service):"},{"line_number":465,"context_line":"        super(ServiceStatusPayload, self).__init__()"},{"line_number":466,"context_line":"        self.single_source_schema()"},{"line_number":467,"context_line":"        self.populate_schema(src\u003dservice)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a122d0e_28d62ef1","line":467,"range":{"start_line":466,"start_character":0,"end_line":467,"end_character":41},"in_reply_to":"1a122d0e_65e7b640","updated":"2016-05-04 12:57:38.000000000","message":"yeah this is really just matter of taste. Let\u0027s go ahead with your proposal. Sorry for nitpicking.","commit_id":"0e096dec15fad7a046b3b1893b91cfbf6e76ce33"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"650888d84094f98d0f79b80cd6564369481cb6d4","unresolved":false,"context_lines":[{"line_number":463,"context_line":""},{"line_number":464,"context_line":"    def __init__(self, service):"},{"line_number":465,"context_line":"        super(ServiceStatusPayload, self).__init__()"},{"line_number":466,"context_line":"        self.single_source_schema()"},{"line_number":467,"context_line":"        self.populate_schema(src\u003dservice)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a122d0e_ff8353a7","line":467,"range":{"start_line":466,"start_character":0,"end_line":467,"end_character":41},"in_reply_to":"1a122d0e_d791419c","updated":"2016-05-04 09:45:56.000000000","message":"can we still use the SCHEMA declaration for the extra fields directly? for example:\n\nSCHEMA \u003d { \u0027host\u0027: (\u0027src\u0027, \u0027dest_host\u0027)}\n\ndef __init__(self, service):\n    super(...)\n    self.populate_fields(src\u003dservice) # this will populate the fields that are available in the service object\n    self.populate_schema(src\u003dservice) # this will use the SCHEMA to do self.host \u003d service.dest_host","commit_id":"0e096dec15fad7a046b3b1893b91cfbf6e76ce33"},{"author":{"_account_id":8688,"name":"Alexis Lee","email":"openstack@lxsli.co.uk","username":"lxsli"},"change_message_id":"eca96703f20a63ffbd9aef01ab3dc91eada95303","unresolved":false,"context_lines":[{"line_number":463,"context_line":""},{"line_number":464,"context_line":"    def __init__(self, service):"},{"line_number":465,"context_line":"        super(ServiceStatusPayload, self).__init__()"},{"line_number":466,"context_line":"        self.single_source_schema()"},{"line_number":467,"context_line":"        self.populate_schema(src\u003dservice)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a122d0e_65e7b640","line":467,"range":{"start_line":466,"start_character":0,"end_line":467,"end_character":41},"in_reply_to":"1a122d0e_ff8353a7","updated":"2016-05-04 10:10:09.000000000","message":"We could I guess but why is that a better solution?","commit_id":"0e096dec15fad7a046b3b1893b91cfbf6e76ce33"}]}