)]}'
{"nova/compute/manager.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e0497ce796910bc79f1c5449d83bee617c41b406","unresolved":false,"context_lines":[{"line_number":1628,"context_line":""},{"line_number":1629,"context_line":"    def _notify_about_instance_usage(self, context, instance, event_suffix,"},{"line_number":1630,"context_line":"                                     network_info\u003dNone, system_metadata\u003dNone,"},{"line_number":1631,"context_line":"                                     extra_usage_info\u003dNone, fault\u003dNone):"},{"line_number":1632,"context_line":"        \"\"\"Send legacy and versioned notifications about an instance."},{"line_number":1633,"context_line":""},{"line_number":1634,"context_line":"        :param:context: security context"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_b4d2a032","line":1631,"range":{"start_line":1631,"start_character":37,"end_line":1631,"end_character":53},"updated":"2016-07-25 13:40:51.000000000","message":"I think this generic dict makes the code unreadable. You can only detect what can it contain by looking through all the place this function is called. I know that this is legacy but I also think in the new notification code path we shall not rely on generic dict that can contain anything but define the parameters explicitly.","commit_id":"179fa7df3cea3a7ebb37d8c17ade4c19bafa8288"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e0497ce796910bc79f1c5449d83bee617c41b406","unresolved":false,"context_lines":[{"line_number":1650,"context_line":"        action, phase \u003d \\"},{"line_number":1651,"context_line":"            fields.parse_notification_action_and_phase(event_suffix)"},{"line_number":1652,"context_line":"        if action and phase:"},{"line_number":1653,"context_line":"            compute_utils.notify_about_instance_action("},{"line_number":1654,"context_line":"                context, instance, self.host, action\u003daction, phase\u003dphase)"},{"line_number":1655,"context_line":""},{"line_number":1656,"context_line":"    def _deallocate_network(self, context, instance,"},{"line_number":1657,"context_line":"                            requested_networks\u003dNone):"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_1463acde","line":1654,"range":{"start_line":1653,"start_character":0,"end_line":1654,"end_character":73},"updated":"2016-07-25 13:40:51.000000000","message":"This will break down as soon as we transform the first notification that extends the InstanceActionPayload class due too the extra_usage_info in the notify_about_instance_usage() call. After that you have to map event type to notification payload class via some magic if you want to keep the code generic. It would be good to see how that will look like before we commit to this direction.","commit_id":"179fa7df3cea3a7ebb37d8c17ade4c19bafa8288"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ba7203f060eb4a9a878bdfed9cdc5ae0f593998d","unresolved":false,"context_lines":[{"line_number":1626,"context_line":"        uuids \u003d [instance.uuid for instance in instances]"},{"line_number":1627,"context_line":"        self.scheduler_client.sync_instance_info(context, self.host, uuids)"},{"line_number":1628,"context_line":""},{"line_number":1629,"context_line":"    def _parse_notification_action_and_phase(self, event_suffix):"},{"line_number":1630,"context_line":"        action, phase \u003d event_suffix.rsplit(\u0027.\u0027, 1)"},{"line_number":1631,"context_line":"        if action not in fields.NotificationAction.ALL:"},{"line_number":1632,"context_line":"            raise exception.NovaException(_(\u0027%s NotificationAction has not \u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"dada55a8_057ce5b3","line":1629,"updated":"2016-07-26 07:28:01.000000000","message":"As I wrote in patch set 4 it is ugly to parse the event_suffix. I would change the manager code to pass in action and phase separately.","commit_id":"dcc3a452488401404ab10693f6915c8460d064df"},{"author":{"_account_id":20411,"name":"Julian","email":"julian.sy@att.com","username":"syjulian"},"change_message_id":"54e434c310e3e8d28177bd8370a2c644b5779181","unresolved":false,"context_lines":[{"line_number":1626,"context_line":"        uuids \u003d [instance.uuid for instance in instances]"},{"line_number":1627,"context_line":"        self.scheduler_client.sync_instance_info(context, self.host, uuids)"},{"line_number":1628,"context_line":""},{"line_number":1629,"context_line":"    def _parse_notification_action_and_phase(self, event_suffix):"},{"line_number":1630,"context_line":"        action, phase \u003d event_suffix.rsplit(\u0027.\u0027, 1)"},{"line_number":1631,"context_line":"        if action not in fields.NotificationAction.ALL:"},{"line_number":1632,"context_line":"            raise exception.NovaException(_(\u0027%s NotificationAction has not \u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"dada55a8_5ff05891","line":1629,"in_reply_to":"dada55a8_057ce5b3","updated":"2016-07-26 13:18:55.000000000","message":"patch set 1 had the manager code pass action and phase and get the event_type, but that would need to change all the calls to _notify_about_instance_usage. Should I go back to doing that?\n\nI did the other way around(pass event_type and get action and phase) so that the refactoring doesn\u0027t change the calls","commit_id":"dcc3a452488401404ab10693f6915c8460d064df"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"c28e3ad0288e6af3fda2088bece217dbeefa75f1","unresolved":false,"context_lines":[{"line_number":1626,"context_line":"        uuids \u003d [instance.uuid for instance in instances]"},{"line_number":1627,"context_line":"        self.scheduler_client.sync_instance_info(context, self.host, uuids)"},{"line_number":1628,"context_line":""},{"line_number":1629,"context_line":"    def _parse_notification_action_and_phase(self, event_suffix):"},{"line_number":1630,"context_line":"        action, phase \u003d event_suffix.rsplit(\u0027.\u0027, 1)"},{"line_number":1631,"context_line":"        if action not in fields.NotificationAction.ALL:"},{"line_number":1632,"context_line":"            raise exception.NovaException(_(\u0027%s NotificationAction has not \u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"dada55a8_ea7afe75","line":1629,"in_reply_to":"dada55a8_5ff05891","updated":"2016-07-27 07:50:58.000000000","message":"Yeah, this is one of my problem with the goal of this refactoring. If we want to be explicit with action and phase and at the same time we want to keep one single function both for legacy and versioned notifications then we have to change the legacy code as that is not explicit about action and phase.\nI personally would keep the legacy and versioned code path separate as much as possible so that the versioned one can be made better (e.g. explicit about parameters) while we don\u0027t have to refactor the legacy one that will be removed in the future.","commit_id":"dcc3a452488401404ab10693f6915c8460d064df"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ba7203f060eb4a9a878bdfed9cdc5ae0f593998d","unresolved":false,"context_lines":[{"line_number":1661,"context_line":"        except (exception.NovaException):"},{"line_number":1662,"context_line":"            pass"},{"line_number":1663,"context_line":"        else:"},{"line_number":1664,"context_line":"            compute_utils.notify_about_instance_action("},{"line_number":1665,"context_line":"                context, instance, self.host, action\u003daction, phase\u003dphase)"},{"line_number":1666,"context_line":""},{"line_number":1667,"context_line":"    def _deallocate_network(self, context, instance,"}],"source_content_type":"text/x-python","patch_set":5,"id":"dada55a8_f817b842","line":1664,"updated":"2016-07-26 07:28:01.000000000","message":"As I wrote in patch set 4 it cannot be kept this generic as soon as we transform some of the notifications that extends the InstanceActionPayload. \n\nAlso I think the extra_usage_info generic dict shall not be used in the new code path to make the code more explicit.","commit_id":"dcc3a452488401404ab10693f6915c8460d064df"}],"nova/objects/fields.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e0497ce796910bc79f1c5449d83bee617c41b406","unresolved":false,"context_lines":[{"line_number":636,"context_line":""},{"line_number":637,"context_line":""},{"line_number":638,"context_line":"def parse_notification_action_and_phase(event_suffix):"},{"line_number":639,"context_line":"    action, phase \u003d event_suffix.rsplit(\u0027.\u0027, 1)"},{"line_number":640,"context_line":"    print(action, phase)"},{"line_number":641,"context_line":"    if action not in NotificationAction.ALL:"},{"line_number":642,"context_line":"        action \u003d None"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_591b9dca","line":639,"updated":"2016-07-25 13:40:51.000000000","message":"It is ugly that we are parsing back what the code in the manager.py passes in. We shall explicitly passing in the action and the phase from the manager.\n\nIt will also break for example in case of \u0027live.migration.abort.start\u0027 event type. (but we might want to rename that to use underscore instead dashes)","commit_id":"179fa7df3cea3a7ebb37d8c17ade4c19bafa8288"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e0497ce796910bc79f1c5449d83bee617c41b406","unresolved":false,"context_lines":[{"line_number":637,"context_line":""},{"line_number":638,"context_line":"def parse_notification_action_and_phase(event_suffix):"},{"line_number":639,"context_line":"    action, phase \u003d event_suffix.rsplit(\u0027.\u0027, 1)"},{"line_number":640,"context_line":"    print(action, phase)"},{"line_number":641,"context_line":"    if action not in NotificationAction.ALL:"},{"line_number":642,"context_line":"        action \u003d None"},{"line_number":643,"context_line":"    if phase not in NotificationPhase.ALL:"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_997b55f4","line":640,"updated":"2016-07-25 13:40:51.000000000","message":"remove debug code","commit_id":"179fa7df3cea3a7ebb37d8c17ade4c19bafa8288"},{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"e0497ce796910bc79f1c5449d83bee617c41b406","unresolved":false,"context_lines":[{"line_number":639,"context_line":"    action, phase \u003d event_suffix.rsplit(\u0027.\u0027, 1)"},{"line_number":640,"context_line":"    print(action, phase)"},{"line_number":641,"context_line":"    if action not in NotificationAction.ALL:"},{"line_number":642,"context_line":"        action \u003d None"},{"line_number":643,"context_line":"    if phase not in NotificationPhase.ALL:"},{"line_number":644,"context_line":"        phase \u003d None"},{"line_number":645,"context_line":"    return action, phase"}],"source_content_type":"text/x-python","patch_set":4,"id":"dada55a8_f98251c7","line":642,"updated":"2016-07-25 13:40:51.000000000","message":"Action cannot be None in the notification.","commit_id":"179fa7df3cea3a7ebb37d8c17ade4c19bafa8288"}],"nova/tests/functional/notification_sample_tests/test_instance.py":[{"author":{"_account_id":9708,"name":"Balazs Gibizer","display_name":"gibi","email":"gibizer@gmail.com","username":"gibi"},"change_message_id":"ba7203f060eb4a9a878bdfed9cdc5ae0f593998d","unresolved":false,"context_lines":[{"line_number":33,"context_line":"        self.api.delete_server(server[\u0027id\u0027])"},{"line_number":34,"context_line":"        self._wait_until_deleted(server)"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"        self.assertEqual(4, len(fake_notifier.VERSIONED_NOTIFICATIONS))"},{"line_number":37,"context_line":"        self._verify_notification("},{"line_number":38,"context_line":"            \u0027instance-delete-start\u0027,"},{"line_number":39,"context_line":"            replacements\u003d{"}],"source_content_type":"text/x-python","patch_set":5,"id":"dada55a8_2575a9f9","line":36,"updated":"2016-07-26 07:28:01.000000000","message":"What kind of notification is coming now? Is it a side effect of the generalization of the code?","commit_id":"dcc3a452488401404ab10693f6915c8460d064df"},{"author":{"_account_id":20411,"name":"Julian","email":"julian.sy@att.com","username":"syjulian"},"change_message_id":"54e434c310e3e8d28177bd8370a2c644b5779181","unresolved":false,"context_lines":[{"line_number":33,"context_line":"        self.api.delete_server(server[\u0027id\u0027])"},{"line_number":34,"context_line":"        self._wait_until_deleted(server)"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"        self.assertEqual(4, len(fake_notifier.VERSIONED_NOTIFICATIONS))"},{"line_number":37,"context_line":"        self._verify_notification("},{"line_number":38,"context_line":"            \u0027instance-delete-start\u0027,"},{"line_number":39,"context_line":"            replacements\u003d{"}],"source_content_type":"text/x-python","patch_set":5,"id":"dada55a8_7fc0bc50","line":36,"in_reply_to":"dada55a8_2575a9f9","updated":"2016-07-26 13:18:55.000000000","message":"_notify_about_instance_usage sends the versioned notification if it\u0027s defined in the NotificationAction Enum.\n\nThe shutdown notifications are coming in between the delete start and end notifications","commit_id":"dcc3a452488401404ab10693f6915c8460d064df"}]}