)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6676,"name":"zqfan","email":"aji.zqfan@gmail.com","username":"aji-zqfan"},"change_message_id":"c4baefd5a36c27e68167a88f83ce21111af38f29","unresolved":false,"context_lines":[{"line_number":11,"context_line":"\"store_events\" option as True in ceilometer.conf."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"This patch catch the NotImplementedError and enable the"},{"line_number":14,"context_line":"notification-to-sample process."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"Change-Id: I7ace5a8581518fa56e58abca2e377b513ff38675"},{"line_number":17,"context_line":"Closes-Bug: #1320420"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"1ae5cdf2_2b0b602c","line":14,"updated":"2014-06-09 09:19:06.000000000","message":"nit: s/catch/catches/, s/enables/enable/","commit_id":"48e30f52d46ce61ca561c271d4c32700a8d27081"}],"ceilometer/event/endpoint.py":[{"author":{"_account_id":6676,"name":"zqfan","email":"aji.zqfan@gmail.com","username":"aji-zqfan"},"change_message_id":"c4baefd5a36c27e68167a88f83ce21111af38f29","unresolved":false,"context_lines":[{"line_number":62,"context_line":""},{"line_number":63,"context_line":"        if event is not None:"},{"line_number":64,"context_line":"            LOG.debug(_(\u0027Saving event \"%s\"\u0027), event.event_type)"},{"line_number":65,"context_line":"            problem_events \u003d []"},{"line_number":66,"context_line":"            for dispatcher_ext in self.dispatcher_manager:"},{"line_number":67,"context_line":"                try:"},{"line_number":68,"context_line":"                    problem_events.extend("}],"source_content_type":"text/x-python","patch_set":4,"id":"1ae5cdf2_7e19a478","line":65,"updated":"2014-06-09 09:19:06.000000000","message":"curious: the name of the variable is a bit odd to me","commit_id":"48e30f52d46ce61ca561c271d4c32700a8d27081"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"3ee7cbc6168153227b6225e4137dbe450da22ffb","unresolved":false,"context_lines":[{"line_number":68,"context_line":"                    problem_events.extend("},{"line_number":69,"context_line":"                        dispatcher_ext.obj.record_events(event))"},{"line_number":70,"context_line":"                except NotImplementedError:"},{"line_number":71,"context_line":"                    LOG.warn(_(\u0027Event is not implemented with the storage\u0027"},{"line_number":72,"context_line":"                               \u0027 backend\u0027))"},{"line_number":73,"context_line":"            if models.Event.UNKNOWN_PROBLEM in [x[0] for x in problem_events]:"},{"line_number":74,"context_line":"                if not cfg.CONF.notification.ack_on_event_error:"}],"source_content_type":"text/x-python","patch_set":4,"id":"1ae5cdf2_1e3980d8","line":71,"updated":"2014-06-09 09:17:06.000000000","message":"Nitpick:\n\n   s/not implemented with/not implemented by/","commit_id":"48e30f52d46ce61ca561c271d4c32700a8d27081"}],"ceilometer/notification.py":[{"author":{"_account_id":6676,"name":"zqfan","email":"aji.zqfan@gmail.com","username":"aji-zqfan"},"change_message_id":"2a143191e8f2357e5014c3a7fb7bde9695cdbe72","unresolved":false,"context_lines":[{"line_number":82,"context_line":"        if cfg.CONF.notification.store_events:"},{"line_number":83,"context_line":"            if (self.storage_conn.record_events.__module__"},{"line_number":84,"context_line":"                    \u003d\u003d \u0027ceilometer.storage.base\u0027):"},{"line_number":85,"context_line":"                LOG.warn(_(\u0027Event is not implemented with storage :%s\u0027),"},{"line_number":86,"context_line":"                         cfg.CONF.database.connection)"},{"line_number":87,"context_line":"            else:"},{"line_number":88,"context_line":"                endpoints \u003d [event_endpoint.EventsNotificationEndpoint()]"}],"source_content_type":"text/x-python","patch_set":2,"id":"1ae5cdf2_bcabc673","line":85,"updated":"2014-06-04 02:29:31.000000000","message":"very tiny nit: storage :%s -\u003e storage: %s\n\nnit: since this may totally disable the notification service, I\u0027m wondering if the warn level is enough, need other reviewers\u0027 opinion","commit_id":"abd3902ca69efdb6cf6a32df7f38751a376b2df4"},{"author":{"_account_id":8290,"name":"liusheng","email":"liusheng2048@gmail.com","username":"LiuSheng"},"change_message_id":"70ade95ce19caac5e27cb693c7504486b3dd3838","unresolved":false,"context_lines":[{"line_number":82,"context_line":"        if cfg.CONF.notification.store_events:"},{"line_number":83,"context_line":"            if (self.storage_conn.record_events.__module__"},{"line_number":84,"context_line":"                    \u003d\u003d \u0027ceilometer.storage.base\u0027):"},{"line_number":85,"context_line":"                LOG.warn(_(\u0027Event is not implemented with storage :%s\u0027),"},{"line_number":86,"context_line":"                         cfg.CONF.database.connection)"},{"line_number":87,"context_line":"            else:"},{"line_number":88,"context_line":"                endpoints \u003d [event_endpoint.EventsNotificationEndpoint()]"}],"source_content_type":"text/x-python","patch_set":2,"id":"1ae5cdf2_ef482d5e","line":85,"in_reply_to":"1ae5cdf2_bcabc673","updated":"2014-06-04 06:24:49.000000000","message":"Done","commit_id":"abd3902ca69efdb6cf6a32df7f38751a376b2df4"},{"author":{"_account_id":2284,"name":"Eoghan Glynn","email":"eglynn@redhat.com","username":"eglynn"},"change_message_id":"1ae3ed843f8409532e2da0fc5461b2fa5b5f8adf","unresolved":false,"context_lines":[{"line_number":81,"context_line":"        endpoints \u003d []"},{"line_number":82,"context_line":"        if cfg.CONF.notification.store_events:"},{"line_number":83,"context_line":"            if (self.storage_conn.record_events.__module__"},{"line_number":84,"context_line":"                    \u003d\u003d \u0027ceilometer.storage.base\u0027):"},{"line_number":85,"context_line":"                LOG.warn(_(\u0027Event is not implemented with storage: %s\u0027),"},{"line_number":86,"context_line":"                         cfg.CONF.database.connection)"},{"line_number":87,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"1ae5cdf2_c3142a46","line":84,"updated":"2014-06-04 08:32:56.000000000","message":"I\u0027m not loving this direct reliance on __module__ \u003d\u003d \u0027ceilometer.storage.base\u0027, seems brittle to me.\n\nWhy not just change the EventsNotificationEndpoint instead, to adopt a suck-it-and-see approach on the call into record_events?\n\ni.e. wrap this call:\n\n  https://github.com/openstack/ceilometer/blob/master/ceilometer/event/endpoint.py#L67\n\nin a try-except-NotImplementedError block, and simply fall out to marking the notification as HANDLED in that case?","commit_id":"4ff21354d398a25d8be355a53bc629e30502bd46"},{"author":{"_account_id":8290,"name":"liusheng","email":"liusheng2048@gmail.com","username":"LiuSheng"},"change_message_id":"d7f64472bf9de7c27f0631dbc079ea3ec55aff8b","unresolved":false,"context_lines":[{"line_number":81,"context_line":"        endpoints \u003d []"},{"line_number":82,"context_line":"        if cfg.CONF.notification.store_events:"},{"line_number":83,"context_line":"            if (self.storage_conn.record_events.__module__"},{"line_number":84,"context_line":"                    \u003d\u003d \u0027ceilometer.storage.base\u0027):"},{"line_number":85,"context_line":"                LOG.warn(_(\u0027Event is not implemented with storage: %s\u0027),"},{"line_number":86,"context_line":"                         cfg.CONF.database.connection)"},{"line_number":87,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"1ae5cdf2_45126259","line":84,"in_reply_to":"1ae5cdf2_4253386c","updated":"2014-06-09 01:50:10.000000000","message":"@Lianhao Lu, thanks for review.\n\n1. OK, I will upload a new patchset according Eglynn\u0027s method.\n\n2. Sorry, I didn\u0027t see your question in bugzilla page. For the question, the listener(only one listener) is base on a list of endpoints which include EventsNotificationEndpoint and others notification endpoints, following is output of my pdb action:\n  p endpoints\n[\u003cceilometer.event.endpoint.EventsNotificationEndpoint object at 0x2f27350\u003e, \u003cceilometer.compute.notifications.instance.InstanceDelete object at 0x2e571d0\u003e, \u003cceilometer.image.notifications.Image object at 0x2e59d50\u003e, \n\u003cceilometer.volume.notifications.VolumeSize object at 0x2f32550\u003e, \u003cceilometer.compute.notifications.instance.InstanceFlavor object at 0x2f325d0\u003e, \n\u003cceilometer.network.notifications.Bandwidth object at 0x2f327d0\u003e, \u003cceilometer.compute.notifications.instance.InstanceScheduled object at 0x2f32850\u003e, \n\u003cceilometer.compute.notifications.cpu.CpuKernelPercent object at 0x2f1fb90\u003e, \u003cceilometer.network.notifications.Port object at 0x2f1fc10\u003e, \u003cceilometer.middleware.HTTPRequest object at 0x2f40150\u003e, \n\u003cceilometer.network.notifications.Network object at 0x2f401d0\u003e, \u003cceilometer.compute.notifications.cpu.CpuPercent object at 0x2f40250\u003e, \n\u003cceilometer.compute.notifications.cpu.CpuIdleTime object at 0x2f402d0\u003e, \u003cceilometer.compute.notifications.instance.VCpus object at 0x2f40350\u003e, \n\u003cceilometer.compute.notifications.cpu.CpuFrequency object at 0x2f403d0\u003e, \u003cceilometer.compute.notifications.instance.Instance object at 0x2f40450\u003e, \n\u003cceilometer.compute.notifications.instance.Memory object at 0x2f404d0\u003e, \u003cceilometer.image.notifications.ImageCRUD object at 0x2f40550\u003e, \u003cceilometer.volume.notifications.Snapshot object at 0x2f405d0\u003e, \n\u003cceilometer.compute.notifications.cpu.CpuUserTime object at 0x2f40650\u003e, \u003cceilometer.compute.notifications.cpu.CpuIdlePercent object at 0x2f406d0\u003e, \u003cceilometer.network.notifications.Subnet object at 0x2f40750\u003e, \n\u003cceilometer.volume.notifications.Volume object at 0x2f407d0\u003e, \u003cceilometer.compute.notifications.cpu.CpuIowaitTime object at 0x2f40850\u003e, \n\u003cceilometer.compute.notifications.cpu.CpuUserPercent object at 0x2f408d0\u003e, \u003cceilometer.image.notifications.ImageSize object at 0x2f40950\u003e, \u003cceilometer.compute.notifications.cpu.CpuKernelTime object at 0x2f409d0\u003e, \n\u003cceilometer.compute.notifications.cpu.CpuIowaitPercent object at 0x2f40a50\u003e, \u003cceilometer.image.notifications.ImageDownload object at 0x2f40ad0\u003e, \n\u003cceilometer.compute.notifications.instance.EphemeralDiskSize object at 0x2f40b50\u003e, \u003cceilometer.network.notifications.FloatingIP object at 0x2f40bd0\u003e, \u003cceilometer.orchestration.notifications.StackCRUD object at 0x2f27050\u003e,\n\u003cceilometer.middleware.HTTPResponse object at 0x2f270d0\u003e, \u003cceilometer.compute.notifications.instance.RootDiskSize object at 0x2f27150\u003e, \n\u003cceilometer.image.notifications.ImageServe object at 0x2f271d0\u003e,\n\u003cceilometer.network.notifications.Router object at 0x2f27250\u003e, \u003cceilometer.volume.notifications.SnapshotSize object at 0x2f272d0\u003e]\n\nThe event endpoint is the first element, if the event endpoint raise an NotImplementedError, \n\nthe endpoints after the event endpoint of the endpoints list will don\u0027t be run.\n\nso the samples will don\u0027t be record. I have tested, if I change the code to :\n          for ext in self.notification_manager:\n            handler \u003d ext.obj\n            LOG.debug(_(\u0027Event types from %(name)s: %(type)s\u0027\n                        \u0027 (ack_on_error\u003d%(error)s)\u0027) %\n                      {\u0027name\u0027: ext.name,\n                       \u0027type\u0027: \u0027, \u0027.join(handler.event_types),\n                       \u0027error\u0027: ack_on_error})\n            targets.extend(handler.get_targets(cfg.CONF))\n            endpoints.append(handler)\n        if cfg.CONF.notification.store_events:\n            endpoints.append(event_endpoint.EventsNotificationEndpoint())\n\nthe samples will be recoded, and the event endpoint will raise exception","commit_id":"4ff21354d398a25d8be355a53bc629e30502bd46"},{"author":{"_account_id":4491,"name":"Lianhao Lu","email":"llh_misc@outlook.com","username":"lianhao-lu"},"change_message_id":"00d725f72467cde23c75263c8ceab4d2bee3945c","unresolved":false,"context_lines":[{"line_number":81,"context_line":"        endpoints \u003d []"},{"line_number":82,"context_line":"        if cfg.CONF.notification.store_events:"},{"line_number":83,"context_line":"            if (self.storage_conn.record_events.__module__"},{"line_number":84,"context_line":"                    \u003d\u003d \u0027ceilometer.storage.base\u0027):"},{"line_number":85,"context_line":"                LOG.warn(_(\u0027Event is not implemented with storage: %s\u0027),"},{"line_number":86,"context_line":"                         cfg.CONF.database.connection)"},{"line_number":87,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"1ae5cdf2_a5b5e632","line":84,"in_reply_to":"1ae5cdf2_45126259","updated":"2014-06-09 01:55:35.000000000","message":"This sounds to me more like an oslo.messaging bug. Maybe asking Mehdi Abaakouk for his comment?","commit_id":"4ff21354d398a25d8be355a53bc629e30502bd46"},{"author":{"_account_id":8290,"name":"liusheng","email":"liusheng2048@gmail.com","username":"LiuSheng"},"change_message_id":"55921bd492abe781eeec38e053d2e26ab38c8d8f","unresolved":false,"context_lines":[{"line_number":81,"context_line":"        endpoints \u003d []"},{"line_number":82,"context_line":"        if cfg.CONF.notification.store_events:"},{"line_number":83,"context_line":"            if (self.storage_conn.record_events.__module__"},{"line_number":84,"context_line":"                    \u003d\u003d \u0027ceilometer.storage.base\u0027):"},{"line_number":85,"context_line":"                LOG.warn(_(\u0027Event is not implemented with storage: %s\u0027),"},{"line_number":86,"context_line":"                         cfg.CONF.database.connection)"},{"line_number":87,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"1ae5cdf2_05e85a7b","line":84,"in_reply_to":"1ae5cdf2_a5b5e632","updated":"2014-06-09 03:14:26.000000000","message":"I am not sure if it is reasonable with oslo.messaging, but ceilometer should also handle the NotImplementedError","commit_id":"4ff21354d398a25d8be355a53bc629e30502bd46"},{"author":{"_account_id":4491,"name":"Lianhao Lu","email":"llh_misc@outlook.com","username":"lianhao-lu"},"change_message_id":"e90450a8881a7967fbf6c386d51747b846abef31","unresolved":false,"context_lines":[{"line_number":81,"context_line":"        endpoints \u003d []"},{"line_number":82,"context_line":"        if cfg.CONF.notification.store_events:"},{"line_number":83,"context_line":"            if (self.storage_conn.record_events.__module__"},{"line_number":84,"context_line":"                    \u003d\u003d \u0027ceilometer.storage.base\u0027):"},{"line_number":85,"context_line":"                LOG.warn(_(\u0027Event is not implemented with storage: %s\u0027),"},{"line_number":86,"context_line":"                         cfg.CONF.database.connection)"},{"line_number":87,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"1ae5cdf2_4253386c","line":84,"in_reply_to":"1ae5cdf2_b7df38d5","updated":"2014-06-09 01:10:14.000000000","message":"I personally prefer Eglynn\u0027s method. It\u0027s price the users have to pay if they enabled the an unsupported feature.\n\nThough I don\u0027t understand why the exception in one notification listener would affect the other notification listeners, see my question in the bugzilla page.","commit_id":"4ff21354d398a25d8be355a53bc629e30502bd46"},{"author":{"_account_id":8290,"name":"liusheng","email":"liusheng2048@gmail.com","username":"LiuSheng"},"change_message_id":"9fe460d3e0880c2136f6897028fdf7f2b2d69d2b","unresolved":false,"context_lines":[{"line_number":81,"context_line":"        endpoints \u003d []"},{"line_number":82,"context_line":"        if cfg.CONF.notification.store_events:"},{"line_number":83,"context_line":"            if (self.storage_conn.record_events.__module__"},{"line_number":84,"context_line":"                    \u003d\u003d \u0027ceilometer.storage.base\u0027):"},{"line_number":85,"context_line":"                LOG.warn(_(\u0027Event is not implemented with storage: %s\u0027),"},{"line_number":86,"context_line":"                         cfg.CONF.database.connection)"},{"line_number":87,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":3,"id":"1ae5cdf2_b7df38d5","line":84,"in_reply_to":"1ae5cdf2_c3142a46","updated":"2014-06-05 01:39:09.000000000","message":"Hi Eoghan, thanks for your review, to fix the bug, I have 3 ideas:\n\n1 use try-except-NotImplementedError block, like you suggested\n    try:\n         problem_events.extend(dispatcher_ext.obj.record_events(event))\n    except NotImplementedError\n           ...\nbut I think it is a bit unreasonable, because every time of processing the notification will run try-except block,\nthis is meaningless, because the event is not implemented. So I abandoned the first idea, \nit better to avoid adding the event endpoint to notification endpoints list.\n\n2 At here, I use try-except-NotImplementedError block like:\n        try:\n            self.storage_conn.record_events(None)\n        except NotImplementedError:\n            ...\nhere I ues *None* as parameter, in the notimplemented-record_event scenario(e.g.mongodb), it will raise NotImplementedError,\nthis can be handled and void add the event endpoint to endpoints list. but in implemented-record_evnets scenario (e.g. mysql), \nit will raise AttributeError (or TypeError) for the \"None\" parameter, this cannot be handled, because in impl_sqlalchemy.py, \nthe record_event() method will accept the *Exception* and don\u0027t raise\n(see:https://github.com/openstack/ceilometer/blob/master/ceilometer/storage/impl_sqlalchemy.py#L929)! \nAnyway, if I use a fake event model as the parameter to instead \"None\", \nthat will record a extra event item to db, it is unreasonable. so abandoned the 2th.\n\n3 use  __module__ \u003d\u003d \u0027ceilometer.storage.base\u0027, I agree with you, it brittle, I hope to find a better way :-)","commit_id":"4ff21354d398a25d8be355a53bc629e30502bd46"}],"ceilometer/tests/test_notification.py":[{"author":{"_account_id":6676,"name":"zqfan","email":"aji.zqfan@gmail.com","username":"aji-zqfan"},"change_message_id":"2a143191e8f2357e5014c3a7fb7bde9695cdbe72","unresolved":false,"context_lines":[{"line_number":94,"context_line":"        self.CONF.set_override(\"connection\", \"log://\", group\u003d\u0027database\u0027)"},{"line_number":95,"context_line":"        self.CONF.set_override(\"store_events\", False, group\u003d\"notification\")"},{"line_number":96,"context_line":"        self.srv \u003d notification.NotificationService()"},{"line_number":97,"context_line":"        setattr(self.srv.storage_conn, \u0027record_events\u0027, lambda func: None)"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"    def fake_get_notifications_manager(self, pm):"},{"line_number":100,"context_line":"        self.plugin \u003d instance.Instance(pm)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1ae5cdf2_1c33da63","line":97,"updated":"2014-06-04 02:29:31.000000000","message":"nit: why add it here, is it necessary for other test in this class, correct me if i\u0027m wrong. moreover, I think mock (or fixture) is better than directly set and del","commit_id":"abd3902ca69efdb6cf6a32df7f38751a376b2df4"},{"author":{"_account_id":8290,"name":"liusheng","email":"liusheng2048@gmail.com","username":"LiuSheng"},"change_message_id":"70ade95ce19caac5e27cb693c7504486b3dd3838","unresolved":false,"context_lines":[{"line_number":94,"context_line":"        self.CONF.set_override(\"connection\", \"log://\", group\u003d\u0027database\u0027)"},{"line_number":95,"context_line":"        self.CONF.set_override(\"store_events\", False, group\u003d\"notification\")"},{"line_number":96,"context_line":"        self.srv \u003d notification.NotificationService()"},{"line_number":97,"context_line":"        setattr(self.srv.storage_conn, \u0027record_events\u0027, lambda func: None)"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"    def fake_get_notifications_manager(self, pm):"},{"line_number":100,"context_line":"        self.plugin \u003d instance.Instance(pm)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1ae5cdf2_af42a53b","line":97,"in_reply_to":"1ae5cdf2_1c33da63","updated":"2014-06-04 06:24:49.000000000","message":"if remove this line of the code, the test_event_dispatcher_loaded() and test_process_notification_with_events() will run failed, because after the changes being applied, the \"log\" storage backend haven\u0027t implemented the record_events() method and, the event endpoint will don\u0027t be add to endpoints list. here make the ceilometer.storage.impl_log.Connection class has \"implemented\" the record_events().\n\n\nIn test_event_dispatcher_loaded_with_record_events_not_impl() should restore the *un-implementation* of record_events()\n\nI wan\u0027t to use mock, but I haven\u0027t find any good way to achieve dynamically adding or deleting a attribute to an object :(","commit_id":"abd3902ca69efdb6cf6a32df7f38751a376b2df4"},{"author":{"_account_id":8290,"name":"liusheng","email":"liusheng2048@gmail.com","username":"LiuSheng"},"change_message_id":"511d7f549b7363052e0f91799f0ae36ec33d49e5","unresolved":false,"context_lines":[{"line_number":94,"context_line":"        self.CONF.set_override(\"connection\", \"log://\", group\u003d\u0027database\u0027)"},{"line_number":95,"context_line":"        self.CONF.set_override(\"store_events\", False, group\u003d\"notification\")"},{"line_number":96,"context_line":"        self.srv \u003d notification.NotificationService()"},{"line_number":97,"context_line":"        setattr(self.srv.storage_conn, \u0027record_events\u0027, lambda func: None)"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"    def fake_get_notifications_manager(self, pm):"},{"line_number":100,"context_line":"        self.plugin \u003d instance.Instance(pm)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1ae5cdf2_9238ae3e","line":97,"in_reply_to":"1ae5cdf2_2fbcd5db","updated":"2014-06-04 07:28:04.000000000","message":"So just keep that as now in PS3(meanwhile, find the usable mock way), if some core members have the same view, I will try to address next PS","commit_id":"abd3902ca69efdb6cf6a32df7f38751a376b2df4"},{"author":{"_account_id":6676,"name":"zqfan","email":"aji.zqfan@gmail.com","username":"aji-zqfan"},"change_message_id":"3e7384ef9fd1f3407c225316891814ce7007d0ef","unresolved":false,"context_lines":[{"line_number":94,"context_line":"        self.CONF.set_override(\"connection\", \"log://\", group\u003d\u0027database\u0027)"},{"line_number":95,"context_line":"        self.CONF.set_override(\"store_events\", False, group\u003d\"notification\")"},{"line_number":96,"context_line":"        self.srv \u003d notification.NotificationService()"},{"line_number":97,"context_line":"        setattr(self.srv.storage_conn, \u0027record_events\u0027, lambda func: None)"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"    def fake_get_notifications_manager(self, pm):"},{"line_number":100,"context_line":"        self.plugin \u003d instance.Instance(pm)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1ae5cdf2_2fbcd5db","line":97,"in_reply_to":"1ae5cdf2_af42a53b","updated":"2014-06-04 06:35:43.000000000","message":"it is just a nit, not very necessary to me. if you do want to use mock, storage.get_connection_from_config can be mocked, then L202 can be removed too","commit_id":"abd3902ca69efdb6cf6a32df7f38751a376b2df4"},{"author":{"_account_id":6676,"name":"zqfan","email":"aji.zqfan@gmail.com","username":"aji-zqfan"},"change_message_id":"2a143191e8f2357e5014c3a7fb7bde9695cdbe72","unresolved":false,"context_lines":[{"line_number":174,"context_line":"        delattr(self.srv.storage_conn, \u0027record_events\u0027)"},{"line_number":175,"context_line":"        with mock.patch.object(self.srv, \u0027_get_notifications_manager\u0027) \\"},{"line_number":176,"context_line":"                as get_nm:"},{"line_number":177,"context_line":"                get_nm.side_effect \u003d self.fake_get_notifications_manager"},{"line_number":178,"context_line":"                self.srv.start()"},{"line_number":179,"context_line":"        self.assertEqual(1, len(self.srv.listeners[0].dispatcher.endpoints))"},{"line_number":180,"context_line":"        setattr(self.srv.storage_conn, \u0027record_events\u0027, lambda func: None)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1ae5cdf2_7cfa1e76","line":177,"updated":"2014-06-04 02:29:31.000000000","message":"nit: I think maybe the 8 spaces indentation is not suitable.","commit_id":"abd3902ca69efdb6cf6a32df7f38751a376b2df4"},{"author":{"_account_id":8290,"name":"liusheng","email":"liusheng2048@gmail.com","username":"LiuSheng"},"change_message_id":"70ade95ce19caac5e27cb693c7504486b3dd3838","unresolved":false,"context_lines":[{"line_number":174,"context_line":"        delattr(self.srv.storage_conn, \u0027record_events\u0027)"},{"line_number":175,"context_line":"        with mock.patch.object(self.srv, \u0027_get_notifications_manager\u0027) \\"},{"line_number":176,"context_line":"                as get_nm:"},{"line_number":177,"context_line":"                get_nm.side_effect \u003d self.fake_get_notifications_manager"},{"line_number":178,"context_line":"                self.srv.start()"},{"line_number":179,"context_line":"        self.assertEqual(1, len(self.srv.listeners[0].dispatcher.endpoints))"},{"line_number":180,"context_line":"        setattr(self.srv.storage_conn, \u0027record_events\u0027, lambda func: None)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1ae5cdf2_0f5d5915","line":177,"in_reply_to":"1ae5cdf2_7cfa1e76","updated":"2014-06-04 06:24:49.000000000","message":"my mistake, Done:)","commit_id":"abd3902ca69efdb6cf6a32df7f38751a376b2df4"},{"author":{"_account_id":6676,"name":"zqfan","email":"aji.zqfan@gmail.com","username":"aji-zqfan"},"change_message_id":"2a143191e8f2357e5014c3a7fb7bde9695cdbe72","unresolved":false,"context_lines":[{"line_number":177,"context_line":"                get_nm.side_effect \u003d self.fake_get_notifications_manager"},{"line_number":178,"context_line":"                self.srv.start()"},{"line_number":179,"context_line":"        self.assertEqual(1, len(self.srv.listeners[0].dispatcher.endpoints))"},{"line_number":180,"context_line":"        setattr(self.srv.storage_conn, \u0027record_events\u0027, lambda func: None)"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"class TestRealNotification(tests_base.BaseTestCase):"}],"source_content_type":"text/x-python","patch_set":2,"id":"1ae5cdf2_7c6f7e2b","line":180,"updated":"2014-06-04 02:29:31.000000000","message":"nit: del then set? I think maybe mock can handle such behavior?","commit_id":"abd3902ca69efdb6cf6a32df7f38751a376b2df4"},{"author":{"_account_id":8290,"name":"liusheng","email":"liusheng2048@gmail.com","username":"LiuSheng"},"change_message_id":"70ade95ce19caac5e27cb693c7504486b3dd3838","unresolved":false,"context_lines":[{"line_number":177,"context_line":"                get_nm.side_effect \u003d self.fake_get_notifications_manager"},{"line_number":178,"context_line":"                self.srv.start()"},{"line_number":179,"context_line":"        self.assertEqual(1, len(self.srv.listeners[0].dispatcher.endpoints))"},{"line_number":180,"context_line":"        setattr(self.srv.storage_conn, \u0027record_events\u0027, lambda func: None)"},{"line_number":181,"context_line":""},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"class TestRealNotification(tests_base.BaseTestCase):"}],"source_content_type":"text/x-python","patch_set":2,"id":"1ae5cdf2_8fa889dc","line":180,"in_reply_to":"1ae5cdf2_7c6f7e2b","updated":"2014-06-04 06:24:49.000000000","message":"I will try to find a way using mock to address that :(","commit_id":"abd3902ca69efdb6cf6a32df7f38751a376b2df4"},{"author":{"_account_id":6676,"name":"zqfan","email":"aji.zqfan@gmail.com","username":"aji-zqfan"},"change_message_id":"2a143191e8f2357e5014c3a7fb7bde9695cdbe72","unresolved":false,"context_lines":[{"line_number":199,"context_line":"        self.CONF.set_override(\"pipeline_cfg_file\", pipeline_cfg_file)"},{"line_number":200,"context_line":"        self.CONF.set_override(\"notification_driver\", \"messaging\")"},{"line_number":201,"context_line":"        self.CONF.set_override(\"control_exchange\", \"nova\")"},{"line_number":202,"context_line":"        self.CONF.set_override(\"connection\", \"log://\", group\u003d\u0027database\u0027)"},{"line_number":203,"context_line":"        messaging.setup(\u0027fake://\u0027)"},{"line_number":204,"context_line":"        self.addCleanup(messaging.cleanup)"},{"line_number":205,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1ae5cdf2_5ca3a24e","line":202,"updated":"2014-06-04 02:29:31.000000000","message":"I don\u0027t know why this line is needed, teach me please :)","commit_id":"abd3902ca69efdb6cf6a32df7f38751a376b2df4"},{"author":{"_account_id":8290,"name":"liusheng","email":"liusheng2048@gmail.com","username":"LiuSheng"},"change_message_id":"70ade95ce19caac5e27cb693c7504486b3dd3838","unresolved":false,"context_lines":[{"line_number":199,"context_line":"        self.CONF.set_override(\"pipeline_cfg_file\", pipeline_cfg_file)"},{"line_number":200,"context_line":"        self.CONF.set_override(\"notification_driver\", \"messaging\")"},{"line_number":201,"context_line":"        self.CONF.set_override(\"control_exchange\", \"nova\")"},{"line_number":202,"context_line":"        self.CONF.set_override(\"connection\", \"log://\", group\u003d\u0027database\u0027)"},{"line_number":203,"context_line":"        messaging.setup(\u0027fake://\u0027)"},{"line_number":204,"context_line":"        self.addCleanup(messaging.cleanup)"},{"line_number":205,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1ae5cdf2_cf6611ed","line":202,"in_reply_to":"1ae5cdf2_5ca3a24e","updated":"2014-06-04 06:24:49.000000000","message":"If the changes applied, the starting of notification service need connection option or raise error.","commit_id":"abd3902ca69efdb6cf6a32df7f38751a376b2df4"},{"author":{"_account_id":6676,"name":"zqfan","email":"aji.zqfan@gmail.com","username":"aji-zqfan"},"change_message_id":"3e7384ef9fd1f3407c225316891814ce7007d0ef","unresolved":false,"context_lines":[{"line_number":199,"context_line":"        self.CONF.set_override(\"pipeline_cfg_file\", pipeline_cfg_file)"},{"line_number":200,"context_line":"        self.CONF.set_override(\"notification_driver\", \"messaging\")"},{"line_number":201,"context_line":"        self.CONF.set_override(\"control_exchange\", \"nova\")"},{"line_number":202,"context_line":"        self.CONF.set_override(\"connection\", \"log://\", group\u003d\u0027database\u0027)"},{"line_number":203,"context_line":"        messaging.setup(\u0027fake://\u0027)"},{"line_number":204,"context_line":"        self.addCleanup(messaging.cleanup)"},{"line_number":205,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1ae5cdf2_ef002dad","line":202,"in_reply_to":"1ae5cdf2_cf6611ed","updated":"2014-06-04 06:35:43.000000000","message":"I see, because the new __init__ will get the real connection...","commit_id":"abd3902ca69efdb6cf6a32df7f38751a376b2df4"}]}