)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d55a5626c927070406813c5f4cde8a86214e70af","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This is the first part of refactoring the L3 agent so that"},{"line_number":10,"context_line":"it uses the new neutron.callbacks mechanism. To do this,"},{"line_number":11,"context_line":"duplicate calls will be made for notifications related to"},{"line_number":12,"context_line":"the router, only using the new callback mechanism."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"As a first change (and since it is in Neutron), the metadata"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9a80dd14_193b2aec","line":11,"updated":"2015-03-18 16:06:33.000000000","message":"not sure I understand the full meaning of \u0027duplicate\u0027. Can you elaborate?","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"87722984ec3a139e53c1b887f266a7e613e39a6e","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This is the first part of refactoring the L3 agent so that"},{"line_number":10,"context_line":"it uses the new neutron.callbacks mechanism. To do this,"},{"line_number":11,"context_line":"duplicate calls will be made for notifications related to"},{"line_number":12,"context_line":"the router, only using the new callback mechanism."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"As a first change (and since it is in Neutron), the metadata"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9a80dd14_a6175553","line":11,"in_reply_to":"9a80dd14_193b2aec","updated":"2015-03-18 18:54:05.000000000","message":"Calls using the new callback mechanism will be added next to calls from the old callback mechanism. Intent is to switch clients over to new mechanism and then delete old ones.","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7c531ea70b6a271366c2d7f358bddef9786e84be","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This is the first part of refactoring the L3 agent so that"},{"line_number":10,"context_line":"it uses the new neutron.callbacks mechanism. To do this,"},{"line_number":11,"context_line":"duplicate calls will be made for notifications related to"},{"line_number":12,"context_line":"the router, only using the new callback mechanism."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"As a first change (and since it is in Neutron), the metadata"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9a80dd14_d76a2cb4","line":11,"in_reply_to":"9a80dd14_a6175553","updated":"2015-03-18 21:14:09.000000000","message":"but then you\u0027d be processing stuff twice, no?","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"fe2eda2bbad17bbe62c4f05ffc093c645f07e941","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This is the first part of refactoring the L3 agent so that"},{"line_number":10,"context_line":"it uses the new neutron.callbacks mechanism. To do this,"},{"line_number":11,"context_line":"duplicate calls will be made for notifications related to"},{"line_number":12,"context_line":"the router, only using the new callback mechanism."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"As a first change (and since it is in Neutron), the metadata"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9a80dd14_eb6572af","line":11,"in_reply_to":"9a80dd14_d76a2cb4","updated":"2015-03-19 12:14:18.000000000","message":"No. With the current mechanism, once a client (e.g. VPNaaS) registers, when each event that occurs, there will be a notification to each registered client, using the method that the client has provided. For VPN, these are specified in vpn_service.py and it overrides the default NOP actions in advanced_service.py. So, if an event is of interest to a client, a specific method will be called to do actions. If the event is not of interest, a NOP method is called.\n\nSo we have VPN and metadata proxy taking actions (either NOP or real action) for each event.\n\nWe are adding a new mechanism, which uses a per event, per client, per client, subscription mechanism. With this commit, we are adding the notifications for the events in L3 agent, subscribing to the new mechanism for the events of interest to metadata proxy, and UNREGISTERING metadata proxy from the old mechanism.\n\nAs a result, when an event occurs, the old mechanism notify will call VPN action (NOP or real). It will not call metadata proxy\u0027s actions, as it is no longer registered. The new mechanism notify will call metadata proxy, only if it has subscribed for that event.\n\nThe approach is to then change each client over to the new mechanism, and then when done, remove the old mechanism (notifiers and classes).","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d55a5626c927070406813c5f4cde8a86214e70af","unresolved":false,"context_lines":[{"line_number":12,"context_line":"the router, only using the new callback mechanism."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"As a first change (and since it is in Neutron), the metadata"},{"line_number":15,"context_line":"proxy is modified to make use of the new mechanism, instead"},{"line_number":16,"context_line":"of the event_notifier for the callbacks. Afterwards, separate"},{"line_number":17,"context_line":"commits will be made in the *aaS repos, and when all of them"},{"line_number":18,"context_line":"are migrated, the old callbacks can be removed."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9a80dd14_7906e622","line":15,"updated":"2015-03-18 16:06:33.000000000","message":"In an initial patch I\u0027d rather see a straight replacement of existing callbacks vs new additions, what do you think?","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7c531ea70b6a271366c2d7f358bddef9786e84be","unresolved":false,"context_lines":[{"line_number":12,"context_line":"the router, only using the new callback mechanism."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"As a first change (and since it is in Neutron), the metadata"},{"line_number":15,"context_line":"proxy is modified to make use of the new mechanism, instead"},{"line_number":16,"context_line":"of the event_notifier for the callbacks. Afterwards, separate"},{"line_number":17,"context_line":"commits will be made in the *aaS repos, and when all of them"},{"line_number":18,"context_line":"are migrated, the old callbacks can be removed."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9a80dd14_57b55c30","line":15,"in_reply_to":"9a80dd14_490dca34","updated":"2015-03-18 21:14:09.000000000","message":"Unless there\u0027s something missing, I think it\u0027s doable. You have one patch that adds the appropriate registry notifications where then need to be; in another patch to take out the old subscriptions and add the new ones.","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"fe2eda2bbad17bbe62c4f05ffc093c645f07e941","unresolved":false,"context_lines":[{"line_number":12,"context_line":"the router, only using the new callback mechanism."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"As a first change (and since it is in Neutron), the metadata"},{"line_number":15,"context_line":"proxy is modified to make use of the new mechanism, instead"},{"line_number":16,"context_line":"of the event_notifier for the callbacks. Afterwards, separate"},{"line_number":17,"context_line":"commits will be made in the *aaS repos, and when all of them"},{"line_number":18,"context_line":"are migrated, the old callbacks can be removed."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9a80dd14_8b6dd634","line":15,"in_reply_to":"9a80dd14_57b55c30","updated":"2015-03-19 12:14:18.000000000","message":"Either you are misunderstanding, or I am totally missing your point. First, I don\u0027t know what you mean by \"registry notifications\". There are notify calls from the old mechanism. I\u0027m adding in new notify() calls for the new mechanism right next to them. The subscriptions are in multiple repos, so they cannot be replaced in one commit.\n\nIn neutron repo , we have the notify() calls in L3 agent, and for the metadata proxy, we have code that registers with the old mechanism and specifies any overridden callbacks (from advanced_services.py, which has ABC with all callbacks for that mechanism).\n\nIn neutron-vpnaas repo, we have code that registers with the old mechanism and specifies ajy overridden callbacks.\n\nThis commit is adding the notifiers for the new mechanism (with only metadata_proxy having subscriptions to the new mechanism, and no longer registered for the old mechanism).\n\nThe next commit, switches neutron-vpnaas repo over to the new mechanism.\n\nThe last commit will remove the old mechanism, now that everyone is switched over.","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"87722984ec3a139e53c1b887f266a7e613e39a6e","unresolved":false,"context_lines":[{"line_number":12,"context_line":"the router, only using the new callback mechanism."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"As a first change (and since it is in Neutron), the metadata"},{"line_number":15,"context_line":"proxy is modified to make use of the new mechanism, instead"},{"line_number":16,"context_line":"of the event_notifier for the callbacks. Afterwards, separate"},{"line_number":17,"context_line":"commits will be made in the *aaS repos, and when all of them"},{"line_number":18,"context_line":"are migrated, the old callbacks can be removed."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9a80dd14_490dca34","line":15,"in_reply_to":"9a80dd14_7906e622","updated":"2015-03-18 18:54:05.000000000","message":"That cannot be done, as the neutron-vpnaas repo uses the old methods.","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"44bf31de6f17bf83755a890a2c90805fe111be5b","unresolved":false,"context_lines":[{"line_number":12,"context_line":"the router, only using the new callback mechanism."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"As a first change (and since it is in Neutron), the metadata"},{"line_number":15,"context_line":"proxy is modified to make use of the new mechanism, instead"},{"line_number":16,"context_line":"of the event_notifier for the callbacks. Afterwards, separate"},{"line_number":17,"context_line":"commits will be made in the *aaS repos, and when all of them"},{"line_number":18,"context_line":"are migrated, the old callbacks can be removed."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9a80dd14_02ee0422","line":15,"in_reply_to":"9a80dd14_8b6dd634","updated":"2015-03-20 00:54:55.000000000","message":"Probably we\u0027re saying the same thing but in different words, leave this as is, and let\u0027s see where we get.","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d55a5626c927070406813c5f4cde8a86214e70af","unresolved":false,"context_lines":[{"line_number":15,"context_line":"proxy is modified to make use of the new mechanism, instead"},{"line_number":16,"context_line":"of the event_notifier for the callbacks. Afterwards, separate"},{"line_number":17,"context_line":"commits will be made in the *aaS repos, and when all of them"},{"line_number":18,"context_line":"are migrated, the old callbacks can be removed."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Change-Id: If134947957fd671aa99a0b2d2b37f7ec65e37766"},{"line_number":21,"context_line":"Partial-Bug: #1433552"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9a80dd14_3d129958","line":18,"updated":"2015-03-18 16:06:33.000000000","message":"I suspect that this needs more coordination, in the sense that we wouldn\u0027t want to process events more than once because the two callback mechanisms coexist for a little lag of time.","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"87722984ec3a139e53c1b887f266a7e613e39a6e","unresolved":false,"context_lines":[{"line_number":15,"context_line":"proxy is modified to make use of the new mechanism, instead"},{"line_number":16,"context_line":"of the event_notifier for the callbacks. Afterwards, separate"},{"line_number":17,"context_line":"commits will be made in the *aaS repos, and when all of them"},{"line_number":18,"context_line":"are migrated, the old callbacks can be removed."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Change-Id: If134947957fd671aa99a0b2d2b37f7ec65e37766"},{"line_number":21,"context_line":"Partial-Bug: #1433552"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9a80dd14_89450245","line":18,"in_reply_to":"9a80dd14_3d129958","updated":"2015-03-18 18:54:05.000000000","message":"That\u0027s why it is using 3 steps. First, adds new mechanism and switches over metadata_proxy, which is in the same repo, to use the new mechanism.  Second, switch over other repos (only VPNaaS currently). Third, remove the old mechanism and notifications, as they are no longer used by anyone. Make sense?","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7c531ea70b6a271366c2d7f358bddef9786e84be","unresolved":false,"context_lines":[{"line_number":15,"context_line":"proxy is modified to make use of the new mechanism, instead"},{"line_number":16,"context_line":"of the event_notifier for the callbacks. Afterwards, separate"},{"line_number":17,"context_line":"commits will be made in the *aaS repos, and when all of them"},{"line_number":18,"context_line":"are migrated, the old callbacks can be removed."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Change-Id: If134947957fd671aa99a0b2d2b37f7ec65e37766"},{"line_number":21,"context_line":"Partial-Bug: #1433552"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9a80dd14_97d0b454","line":18,"in_reply_to":"9a80dd14_89450245","updated":"2015-03-18 21:14:09.000000000","message":"I think this can be a two-step process.","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"fe2eda2bbad17bbe62c4f05ffc093c645f07e941","unresolved":false,"context_lines":[{"line_number":15,"context_line":"proxy is modified to make use of the new mechanism, instead"},{"line_number":16,"context_line":"of the event_notifier for the callbacks. Afterwards, separate"},{"line_number":17,"context_line":"commits will be made in the *aaS repos, and when all of them"},{"line_number":18,"context_line":"are migrated, the old callbacks can be removed."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"Change-Id: If134947957fd671aa99a0b2d2b37f7ec65e37766"},{"line_number":21,"context_line":"Partial-Bug: #1433552"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"9a80dd14_cb734e14","line":18,"in_reply_to":"9a80dd14_97d0b454","updated":"2015-03-19 12:14:18.000000000","message":"You\u0027re gonna need to provide more details on how this could possibly be done in two steps. I don\u0027t see how that can occur.","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"}],"neutron/agent/l3/agent.py":[{"author":{"_account_id":8124,"name":"cbrandily","email":"zzelle@gmail.com","username":"cbrandily"},"change_message_id":"75a831c9473856bf9c68792f43d672f43cf24021","unresolved":false,"context_lines":[{"line_number":294,"context_line":"                                    ri.get_internal_device_name)"},{"line_number":295,"context_line":"        self.event_observers.notify("},{"line_number":296,"context_line":"            adv_svc.AdvancedService.before_router_added, ri)"},{"line_number":297,"context_line":"        kwargs \u003d {\u0027router\u0027: ri}"},{"line_number":298,"context_line":"        registry.notify(resources.ROUTER, events.BEFORE_CREATE, self, **kwargs)"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"        self.router_info[router_id] \u003d ri"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a80dd14_65f7615a","line":297,"updated":"2015-03-17 12:56:08.000000000","message":"kwargs can be inlined everywhere","commit_id":"91a2606dae06ce3e893abd9b0482ab7b88a69717"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"c883ffe3a11b3ccc46b1ea61a7b4ffaf5faae81b","unresolved":false,"context_lines":[{"line_number":294,"context_line":"                                    ri.get_internal_device_name)"},{"line_number":295,"context_line":"        self.event_observers.notify("},{"line_number":296,"context_line":"            adv_svc.AdvancedService.before_router_added, ri)"},{"line_number":297,"context_line":"        kwargs \u003d {\u0027router\u0027: ri}"},{"line_number":298,"context_line":"        registry.notify(resources.ROUTER, events.BEFORE_CREATE, self, **kwargs)"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"        self.router_info[router_id] \u003d ri"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a80dd14_d818eb77","line":297,"in_reply_to":"9a80dd14_65f7615a","updated":"2015-03-17 14:48:20.000000000","message":"+1","commit_id":"91a2606dae06ce3e893abd9b0482ab7b88a69717"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"9b7811d56132b4716631682a817b04342d9dfff7","unresolved":false,"context_lines":[{"line_number":294,"context_line":"                                    ri.get_internal_device_name)"},{"line_number":295,"context_line":"        self.event_observers.notify("},{"line_number":296,"context_line":"            adv_svc.AdvancedService.before_router_added, ri)"},{"line_number":297,"context_line":"        kwargs \u003d {\u0027router\u0027: ri}"},{"line_number":298,"context_line":"        registry.notify(resources.ROUTER, events.BEFORE_CREATE, self, **kwargs)"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"        self.router_info[router_id] \u003d ri"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a80dd14_a9abeb73","line":297,"in_reply_to":"9a80dd14_d818eb77","updated":"2015-03-17 15:30:02.000000000","message":"Done","commit_id":"91a2606dae06ce3e893abd9b0482ab7b88a69717"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"734e14389b5099dc4efb04f5aba6e9639a1ba470","unresolved":false,"context_lines":[{"line_number":293,"context_line":"                                    self.process_monitor,"},{"line_number":294,"context_line":"                                    ri.get_internal_device_name)"},{"line_number":295,"context_line":"        self.event_observers.notify("},{"line_number":296,"context_line":"            adv_svc.AdvancedService.before_router_added, ri)"},{"line_number":297,"context_line":"        registry.notify(resources.ROUTER, events.BEFORE_CREATE,"},{"line_number":298,"context_line":"                        self, **{\u0027router\u0027: ri})"},{"line_number":299,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_48cfc057","line":296,"updated":"2015-03-17 17:56:25.000000000","message":"You can use call notify like this:\nregistry.notify(resources.ROUTER, events.BEFORE_CREATE, router\u003dri). That\u0027s what expanding the dict with ** does.","commit_id":"6cc873d2082f51bf8f9f555779b7b4b5ef84e2ea"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"71a66a56dbe121a08c00cf88ab181810dd9fb379","unresolved":false,"context_lines":[{"line_number":293,"context_line":"                                    self.process_monitor,"},{"line_number":294,"context_line":"                                    ri.get_internal_device_name)"},{"line_number":295,"context_line":"        self.event_observers.notify("},{"line_number":296,"context_line":"            adv_svc.AdvancedService.before_router_added, ri)"},{"line_number":297,"context_line":"        registry.notify(resources.ROUTER, events.BEFORE_CREATE,"},{"line_number":298,"context_line":"                        self, **{\u0027router\u0027: ri})"},{"line_number":299,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_4824200e","line":296,"in_reply_to":"9a80dd14_48cfc057","updated":"2015-03-17 17:58:26.000000000","message":"Done","commit_id":"6cc873d2082f51bf8f9f555779b7b4b5ef84e2ea"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d55a5626c927070406813c5f4cde8a86214e70af","unresolved":false,"context_lines":[{"line_number":711,"context_line":"        self.process_router(ri)"},{"line_number":712,"context_line":"        self.event_observers.notify("},{"line_number":713,"context_line":"            adv_svc.AdvancedService.after_router_added, ri)"},{"line_number":714,"context_line":"        registry.notify(resources.ROUTER, events.AFTER_CREATE, self, router\u003dri)"},{"line_number":715,"context_line":""},{"line_number":716,"context_line":"    def _process_updated_router(self, router):"},{"line_number":717,"context_line":"        # TODO(pcm): Next refactoring will rework this logic"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_fd1f2159","line":714,"updated":"2015-03-18 16:06:33.000000000","message":"I\u0027d rather keep BEFORE_CREATE and AFTER_CREATE close to each other, if possible. Was not feasible at line 306?\n\nThat said, I\u0027d start off with a patch that replace existing functionality rather than adding new one...but I can go along with this.","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"fe2eda2bbad17bbe62c4f05ffc093c645f07e941","unresolved":false,"context_lines":[{"line_number":711,"context_line":"        self.process_router(ri)"},{"line_number":712,"context_line":"        self.event_observers.notify("},{"line_number":713,"context_line":"            adv_svc.AdvancedService.after_router_added, ri)"},{"line_number":714,"context_line":"        registry.notify(resources.ROUTER, events.AFTER_CREATE, self, router\u003dri)"},{"line_number":715,"context_line":""},{"line_number":716,"context_line":"    def _process_updated_router(self, router):"},{"line_number":717,"context_line":"        # TODO(pcm): Next refactoring will rework this logic"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_c630a5eb","line":714,"in_reply_to":"9a80dd14_77b7c021","updated":"2015-03-19 12:14:18.000000000","message":"Regarding moving the location of the notification, that is outside of the scope of this refactoring and can be addressed in a bug that attempts (again) to move the location.\n\nYou\u0027ll need to elaborate on how we can replace the notify, when we have clients (e.g. VPN) in another repository that are using the current notify() call.","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7c531ea70b6a271366c2d7f358bddef9786e84be","unresolved":false,"context_lines":[{"line_number":711,"context_line":"        self.process_router(ri)"},{"line_number":712,"context_line":"        self.event_observers.notify("},{"line_number":713,"context_line":"            adv_svc.AdvancedService.after_router_added, ri)"},{"line_number":714,"context_line":"        registry.notify(resources.ROUTER, events.AFTER_CREATE, self, router\u003dri)"},{"line_number":715,"context_line":""},{"line_number":716,"context_line":"    def _process_updated_router(self, router):"},{"line_number":717,"context_line":"        # TODO(pcm): Next refactoring will rework this logic"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_77b7c021","line":714,"in_reply_to":"9a80dd14_89e1822c","updated":"2015-03-18 21:14:09.000000000","message":"not sure I really agree with your point, I need to think about this some more...","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"44bf31de6f17bf83755a890a2c90805fe111be5b","unresolved":false,"context_lines":[{"line_number":711,"context_line":"        self.process_router(ri)"},{"line_number":712,"context_line":"        self.event_observers.notify("},{"line_number":713,"context_line":"            adv_svc.AdvancedService.after_router_added, ri)"},{"line_number":714,"context_line":"        registry.notify(resources.ROUTER, events.AFTER_CREATE, self, router\u003dri)"},{"line_number":715,"context_line":""},{"line_number":716,"context_line":"    def _process_updated_router(self, router):"},{"line_number":717,"context_line":"        # TODO(pcm): Next refactoring will rework this logic"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_02d52442","line":714,"in_reply_to":"9a80dd14_c630a5eb","updated":"2015-03-20 00:54:55.000000000","message":"ic","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"87722984ec3a139e53c1b887f266a7e613e39a6e","unresolved":false,"context_lines":[{"line_number":711,"context_line":"        self.process_router(ri)"},{"line_number":712,"context_line":"        self.event_observers.notify("},{"line_number":713,"context_line":"            adv_svc.AdvancedService.after_router_added, ri)"},{"line_number":714,"context_line":"        registry.notify(resources.ROUTER, events.AFTER_CREATE, self, router\u003dri)"},{"line_number":715,"context_line":""},{"line_number":716,"context_line":"    def _process_updated_router(self, router):"},{"line_number":717,"context_line":"        # TODO(pcm): Next refactoring will rework this logic"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_89e1822c","line":714,"in_reply_to":"9a80dd14_fd1f2159","updated":"2015-03-18 18:54:05.000000000","message":"That\u0027s what I wanted to do with the original mechanism, however, reviewers disagreed. The intent here, is not to change the notification points, but only to replace them with new ones (hence they are adjacent).  As mentioned in the commit message response, it cannot be immediately replaced, due to cross project dependencies.","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":10980,"name":"Doug Wiegley","email":"dougwig@parkside.io","username":"dougw"},"change_message_id":"59d98749f145ad7e3b5167e2e9638d055a2aca42","unresolved":false,"context_lines":[{"line_number":303,"context_line":"        self.event_observers.notify("},{"line_number":304,"context_line":"            adv_svc.AdvancedService.before_router_added, ri)"},{"line_number":305,"context_line":"        registry.notify(resources.ROUTER, events.BEFORE_CREATE,"},{"line_number":306,"context_line":"                        self, router\u003dri)"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":"        self.router_info[router_id] \u003d ri"},{"line_number":309,"context_line":"        ri.create()"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa963961_d587cc5f","line":306,"updated":"2015-03-27 16:04:21.000000000","message":"So if I look at this addition and the statement above it, they\u0027re both basically, \"notify for some event.\"  Why the duplication, is one going the away at some point, and if not, why is one not implementing the other?","commit_id":"8c5b4aa9109ee333aef219ae5b4e12fb4a4718a9"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"cd290f4f8d15f2fb6444e69ecc3c48aff51001be","unresolved":false,"context_lines":[{"line_number":303,"context_line":"        self.event_observers.notify("},{"line_number":304,"context_line":"            adv_svc.AdvancedService.before_router_added, ri)"},{"line_number":305,"context_line":"        registry.notify(resources.ROUTER, events.BEFORE_CREATE,"},{"line_number":306,"context_line":"                        self, router\u003dri)"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":"        self.router_info[router_id] \u003d ri"},{"line_number":309,"context_line":"        ri.create()"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa963961_15e53436","line":306,"in_reply_to":"fa963961_d587cc5f","updated":"2015-03-27 16:07:19.000000000","message":"The above is old, and because used by other repos, we have to add, then remove later.","commit_id":"8c5b4aa9109ee333aef219ae5b4e12fb4a4718a9"}],"neutron/agent/metadata/driver.py":[{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"c883ffe3a11b3ccc46b1ea61a7b4ffaf5faae81b","unresolved":false,"context_lines":[{"line_number":142,"context_line":"            namespace\u003dns_name,"},{"line_number":143,"context_line":"            default_cmd_callback\u003dcallback)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"def after_router_added(resource, event, l3_agent, **kwargs):"},{"line_number":147,"context_line":"    router \u003d kwargs[\u0027router\u0027]"},{"line_number":148,"context_line":"    proxy \u003d l3_agent.metadata_driver"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a80dd14_bb5331e1","line":145,"updated":"2015-03-17 14:48:20.000000000","message":"It\u0027s a shame the registry doesn\u0027t support class methods at this time, you get in to these awkward situations... Someone should really look in to that, if only this were an open source project and any random person could fix this issue ;-)","commit_id":"91a2606dae06ce3e893abd9b0482ab7b88a69717"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"9b7811d56132b4716631682a817b04342d9dfff7","unresolved":false,"context_lines":[{"line_number":142,"context_line":"            namespace\u003dns_name,"},{"line_number":143,"context_line":"            default_cmd_callback\u003dcallback)"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":""},{"line_number":146,"context_line":"def after_router_added(resource, event, l3_agent, **kwargs):"},{"line_number":147,"context_line":"    router \u003d kwargs[\u0027router\u0027]"},{"line_number":148,"context_line":"    proxy \u003d l3_agent.metadata_driver"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a80dd14_09087790","line":145,"in_reply_to":"9a80dd14_bb5331e1","updated":"2015-03-17 15:30:02.000000000","message":"I think Armando is planning on looking into that more.","commit_id":"91a2606dae06ce3e893abd9b0482ab7b88a69717"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d55a5626c927070406813c5f4cde8a86214e70af","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        self.metadata_port \u003d l3_agent.conf.metadata_port"},{"line_number":58,"context_line":"        self.metadata_access_mark \u003d l3_agent.conf.metadata_access_mark"},{"line_number":59,"context_line":"        registry.subscribe("},{"line_number":60,"context_line":"            after_router_added, resources.ROUTER, events.AFTER_CREATE)"},{"line_number":61,"context_line":"        registry.subscribe("},{"line_number":62,"context_line":"            before_router_removed, resources.ROUTER, events.BEFORE_DELETE)"},{"line_number":63,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_1d44f519","line":60,"updated":"2015-03-18 16:06:33.000000000","message":"you notify more than these two events, are you sure you want to subscribe just to these? Was that done just for completeness?","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"44bf31de6f17bf83755a890a2c90805fe111be5b","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        self.metadata_port \u003d l3_agent.conf.metadata_port"},{"line_number":58,"context_line":"        self.metadata_access_mark \u003d l3_agent.conf.metadata_access_mark"},{"line_number":59,"context_line":"        registry.subscribe("},{"line_number":60,"context_line":"            after_router_added, resources.ROUTER, events.AFTER_CREATE)"},{"line_number":61,"context_line":"        registry.subscribe("},{"line_number":62,"context_line":"            before_router_removed, resources.ROUTER, events.BEFORE_DELETE)"},{"line_number":63,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_2224e85e","line":60,"in_reply_to":"9a80dd14_06d47d1e","updated":"2015-03-20 00:54:55.000000000","message":"The root of my confusion is that this patch is trying to do more than one thing at once, but now I think I understood.","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"4ce50f7e0752a755714259db8e8e1f81dd5c6d30","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        self.metadata_port \u003d l3_agent.conf.metadata_port"},{"line_number":58,"context_line":"        self.metadata_access_mark \u003d l3_agent.conf.metadata_access_mark"},{"line_number":59,"context_line":"        registry.subscribe("},{"line_number":60,"context_line":"            after_router_added, resources.ROUTER, events.AFTER_CREATE)"},{"line_number":61,"context_line":"        registry.subscribe("},{"line_number":62,"context_line":"            before_router_removed, resources.ROUTER, events.BEFORE_DELETE)"},{"line_number":63,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_603b8ac8","line":60,"in_reply_to":"9a80dd14_1d44f519","updated":"2015-03-18 16:11:46.000000000","message":"Paul subscribed this driver to the same events it was subscribed to with the old systems. I don\u0027t understand your concern.","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7c531ea70b6a271366c2d7f358bddef9786e84be","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        self.metadata_port \u003d l3_agent.conf.metadata_port"},{"line_number":58,"context_line":"        self.metadata_access_mark \u003d l3_agent.conf.metadata_access_mark"},{"line_number":59,"context_line":"        registry.subscribe("},{"line_number":60,"context_line":"            after_router_added, resources.ROUTER, events.AFTER_CREATE)"},{"line_number":61,"context_line":"        registry.subscribe("},{"line_number":62,"context_line":"            before_router_removed, resources.ROUTER, events.BEFORE_DELETE)"},{"line_number":63,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_b763f8af","line":60,"in_reply_to":"9a80dd14_29f896c3","updated":"2015-03-18 21:14:09.000000000","message":"what I meant is that he added notifications to all the events but only subscribed to half of it. I\u0027d rather add only the stuff we need.","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"87722984ec3a139e53c1b887f266a7e613e39a6e","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        self.metadata_port \u003d l3_agent.conf.metadata_port"},{"line_number":58,"context_line":"        self.metadata_access_mark \u003d l3_agent.conf.metadata_access_mark"},{"line_number":59,"context_line":"        registry.subscribe("},{"line_number":60,"context_line":"            after_router_added, resources.ROUTER, events.AFTER_CREATE)"},{"line_number":61,"context_line":"        registry.subscribe("},{"line_number":62,"context_line":"            before_router_removed, resources.ROUTER, events.BEFORE_DELETE)"},{"line_number":63,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_29f896c3","line":60,"in_reply_to":"9a80dd14_603b8ac8","updated":"2015-03-18 18:54:05.000000000","message":"Armando, these are the only two events for which the metadata proxy is interested in. Like Assaf says, same as before.","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"fe2eda2bbad17bbe62c4f05ffc093c645f07e941","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        self.metadata_port \u003d l3_agent.conf.metadata_port"},{"line_number":58,"context_line":"        self.metadata_access_mark \u003d l3_agent.conf.metadata_access_mark"},{"line_number":59,"context_line":"        registry.subscribe("},{"line_number":60,"context_line":"            after_router_added, resources.ROUTER, events.AFTER_CREATE)"},{"line_number":61,"context_line":"        registry.subscribe("},{"line_number":62,"context_line":"            before_router_removed, resources.ROUTER, events.BEFORE_DELETE)"},{"line_number":63,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_06d47d1e","line":60,"in_reply_to":"9a80dd14_b763f8af","updated":"2015-03-19 12:14:18.000000000","message":"Maybe you are misunderstanding the mechanism?\n\nThe L3 agent (currently and with the new mechanism) does notifications for each event (before router create, after router create, before router delete, after router delete, before router update, after router update).\n\nEach client (e.g. VPN, metadata proxy) subscribe only to the events they are interested in. In this case, metadata proxy is only interested in the after create and before delete events.  VPN is interested in after create, after remove, and after update, and with the new mechanism will subscribe to those events. VPN currently \"registers\" for all notifications, under the old mechanism, but does NOPs for all but those three events.\n\nNote the end results of the two mechanisms is the same, but they get there two different ways (old -\u003e register for all, implement actions only for interested, new -\u003e subscribe only to events interested in).","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"}],"neutron/tests/functional/agent/test_l3_agent.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"9e6a77093d9380467de8ab9350f6dc93cf1d78d9","unresolved":false,"context_lines":[{"line_number":55,"context_line":"    def setUp(self):"},{"line_number":56,"context_line":"        super(L3AgentTestFramework, self).setUp()"},{"line_number":57,"context_line":"        mock.patch(\u0027neutron.agent.l3.agent.L3PluginApi\u0027).start()"},{"line_number":58,"context_line":"        # Because caallbacks are managed by a singleton, clear them to prevent"},{"line_number":59,"context_line":"        # test interaction"},{"line_number":60,"context_line":"        registry.clear()"},{"line_number":61,"context_line":"        self.agent \u003d self._configure_agent(\u0027agent1\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a80dd14_4e5c1e1a","line":58,"updated":"2015-03-17 21:57:48.000000000","message":"nit: caallbacks, but only fix if respin","commit_id":"eb112ce715431145576668fcff35c414a9de6241"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"fe7f769a56402adcb0f672518f3ebc45f37f2bdb","unresolved":false,"context_lines":[{"line_number":55,"context_line":"    def setUp(self):"},{"line_number":56,"context_line":"        super(L3AgentTestFramework, self).setUp()"},{"line_number":57,"context_line":"        mock.patch(\u0027neutron.agent.l3.agent.L3PluginApi\u0027).start()"},{"line_number":58,"context_line":"        # Because caallbacks are managed by a singleton, clear them to prevent"},{"line_number":59,"context_line":"        # test interaction"},{"line_number":60,"context_line":"        registry.clear()"},{"line_number":61,"context_line":"        self.agent \u003d self._configure_agent(\u0027agent1\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"9a80dd14_7f4a5328","line":58,"in_reply_to":"9a80dd14_4e5c1e1a","updated":"2015-03-18 12:12:20.000000000","message":"It\u0027s spelled that way, because it goes waaay back :)","commit_id":"eb112ce715431145576668fcff35c414a9de6241"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d55a5626c927070406813c5f4cde8a86214e70af","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        super(L3AgentTestFramework, self).setUp()"},{"line_number":57,"context_line":"        mock.patch(\u0027neutron.agent.l3.agent.L3PluginApi\u0027).start()"},{"line_number":58,"context_line":"        # Because callbacks are managed by a singleton, clear them to prevent"},{"line_number":59,"context_line":"        # test interaction"},{"line_number":60,"context_line":"        registry.clear()"},{"line_number":61,"context_line":"        self.agent \u003d self._configure_agent(\u0027agent1\u0027)"},{"line_number":62,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_7df091c9","line":59,"updated":"2015-03-18 16:06:33.000000000","message":"I think we should mock the registry here. That\u0027s how I did it elsewhere in the  testcase, we\u0027d want to minimize side-effect interaction. Clearing the registry is fine, actually, but if we\u0027re not prescriptive about this we\u0027re bound to forget once and we\u0027ll get burned ;)","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"4f9c8f67b74f5a1fae462863bc8fd1e00924d872","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        super(L3AgentTestFramework, self).setUp()"},{"line_number":57,"context_line":"        mock.patch(\u0027neutron.agent.l3.agent.L3PluginApi\u0027).start()"},{"line_number":58,"context_line":"        # Because callbacks are managed by a singleton, clear them to prevent"},{"line_number":59,"context_line":"        # test interaction"},{"line_number":60,"context_line":"        registry.clear()"},{"line_number":61,"context_line":"        self.agent \u003d self._configure_agent(\u0027agent1\u0027)"},{"line_number":62,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_9a6407b4","line":59,"in_reply_to":"9a80dd14_42de3c3f","updated":"2015-03-20 12:18:42.000000000","message":"Fair enough. Will update.","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"834d74a3e8c027a68474c16e93ffcaabeccee71d","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        super(L3AgentTestFramework, self).setUp()"},{"line_number":57,"context_line":"        mock.patch(\u0027neutron.agent.l3.agent.L3PluginApi\u0027).start()"},{"line_number":58,"context_line":"        # Because callbacks are managed by a singleton, clear them to prevent"},{"line_number":59,"context_line":"        # test interaction"},{"line_number":60,"context_line":"        registry.clear()"},{"line_number":61,"context_line":"        self.agent \u003d self._configure_agent(\u0027agent1\u0027)"},{"line_number":62,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_86f5e452","line":59,"in_reply_to":"9a80dd14_57ff1cab","updated":"2015-03-18 23:25:20.000000000","message":"see IRC conversation here:\n\nhttp://eavesdrop.openstack.org/irclogs/%23openstack-neutron/%23openstack-neutron.2015-03-18.log\n\nStarting 2015-03-18T23:04:17","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7c531ea70b6a271366c2d7f358bddef9786e84be","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        super(L3AgentTestFramework, self).setUp()"},{"line_number":57,"context_line":"        mock.patch(\u0027neutron.agent.l3.agent.L3PluginApi\u0027).start()"},{"line_number":58,"context_line":"        # Because callbacks are managed by a singleton, clear them to prevent"},{"line_number":59,"context_line":"        # test interaction"},{"line_number":60,"context_line":"        registry.clear()"},{"line_number":61,"context_line":"        self.agent \u003d self._configure_agent(\u0027agent1\u0027)"},{"line_number":62,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_57ff1cab","line":59,"in_reply_to":"9a80dd14_7d05f112","updated":"2015-03-18 21:14:09.000000000","message":"What I am saying is that in these tests we shouldn\u0027t be using a live instance of the registry, we should used a mocked instance.","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"f2c90f127246659e245f82d8070afbd764ebc178","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        super(L3AgentTestFramework, self).setUp()"},{"line_number":57,"context_line":"        mock.patch(\u0027neutron.agent.l3.agent.L3PluginApi\u0027).start()"},{"line_number":58,"context_line":"        # Because callbacks are managed by a singleton, clear them to prevent"},{"line_number":59,"context_line":"        # test interaction"},{"line_number":60,"context_line":"        registry.clear()"},{"line_number":61,"context_line":"        self.agent \u003d self._configure_agent(\u0027agent1\u0027)"},{"line_number":62,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_7d05f112","line":59,"in_reply_to":"9a80dd14_7df091c9","updated":"2015-03-18 16:10:13.000000000","message":"Umm, clearing the registry will fail most of the tests since the metadata proxy won\u0027t be started / stopped. That should answer your question about coverage.","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"44bf31de6f17bf83755a890a2c90805fe111be5b","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        super(L3AgentTestFramework, self).setUp()"},{"line_number":57,"context_line":"        mock.patch(\u0027neutron.agent.l3.agent.L3PluginApi\u0027).start()"},{"line_number":58,"context_line":"        # Because callbacks are managed by a singleton, clear them to prevent"},{"line_number":59,"context_line":"        # test interaction"},{"line_number":60,"context_line":"        registry.clear()"},{"line_number":61,"context_line":"        self.agent \u003d self._configure_agent(\u0027agent1\u0027)"},{"line_number":62,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_42de3c3f","line":59,"in_reply_to":"9a80dd14_81c4d792","updated":"2015-03-20 00:54:55.000000000","message":"I think that mocking is to be preferred, and I am one reviewer who makes this suggestion as a precondition for the removal of my -1 :)","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"fe2eda2bbad17bbe62c4f05ffc093c645f07e941","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        super(L3AgentTestFramework, self).setUp()"},{"line_number":57,"context_line":"        mock.patch(\u0027neutron.agent.l3.agent.L3PluginApi\u0027).start()"},{"line_number":58,"context_line":"        # Because callbacks are managed by a singleton, clear them to prevent"},{"line_number":59,"context_line":"        # test interaction"},{"line_number":60,"context_line":"        registry.clear()"},{"line_number":61,"context_line":"        self.agent \u003d self._configure_agent(\u0027agent1\u0027)"},{"line_number":62,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_81c4d792","line":59,"in_reply_to":"9a80dd14_86f5e452","updated":"2015-03-19 12:14:18.000000000","message":"OK, so from this IRC discussion, if I understand, you are suggesting that we mock _get_callback_manager(), so that each test gets its own instance, instead of clearing all subscriptions for the singleton.\n\nI guess it\u0027ll have the same effect, only via using a mock vs reinitializing. Is that what we want to do here?","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"40cac9e31a0ff9bd6798806ecc7c0a0fb1b1a578","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        mock.patch(\u0027neutron.agent.l3.agent.L3PluginApi\u0027).start()"},{"line_number":58,"context_line":"        # Because callbacks are managed by a singleton, clear them to prevent"},{"line_number":59,"context_line":"        # test interaction"},{"line_number":60,"context_line":"        registry.clear()"},{"line_number":61,"context_line":"        self.agent \u003d self._configure_agent(\u0027agent1\u0027)"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    def _get_config_opts(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_202fdc3f","line":60,"updated":"2015-03-18 13:21:42.000000000","message":"If another test case is running concurrently with active callbacks, won\u0027t this clear that test\u0027s callbacks?","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"770af44b2b66e9fd9c1f210ce4aa536e2d58fb98","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        mock.patch(\u0027neutron.agent.l3.agent.L3PluginApi\u0027).start()"},{"line_number":58,"context_line":"        # Because callbacks are managed by a singleton, clear them to prevent"},{"line_number":59,"context_line":"        # test interaction"},{"line_number":60,"context_line":"        registry.clear()"},{"line_number":61,"context_line":"        self.agent \u003d self._configure_agent(\u0027agent1\u0027)"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    def _get_config_opts(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_ca4c5a79","line":60,"in_reply_to":"9a80dd14_202fdc3f","updated":"2015-03-18 14:08:09.000000000","message":"The way testr achieves concurrency is with multiple workers, where each worker is a process that runs its share of the tests sequentially. You can\u0027t have the same process run two tests at the same time.","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"8e012eebed6f1ac684f9b3e4d16e1d546d55b765","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        mock.patch(\u0027neutron.agent.l3.agent.L3PluginApi\u0027).start()"},{"line_number":58,"context_line":"        # Because callbacks are managed by a singleton, clear them to prevent"},{"line_number":59,"context_line":"        # test interaction"},{"line_number":60,"context_line":"        registry.clear()"},{"line_number":61,"context_line":"        self.agent \u003d self._configure_agent(\u0027agent1\u0027)"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"    def _get_config_opts(self):"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_0a9f5204","line":60,"in_reply_to":"9a80dd14_ca4c5a79","updated":"2015-03-18 14:23:11.000000000","message":"Right, I knew that. Not sure what I was thinking. Thanks Assaf.","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"e873bf6aac468815d7f9c5b79373e0e117f02c01","unresolved":false,"context_lines":[{"line_number":55,"context_line":"class L3AgentTestFramework(base.BaseOVSLinuxTestCase):"},{"line_number":56,"context_line":"    def setUp(self):"},{"line_number":57,"context_line":"        super(L3AgentTestFramework, self).setUp()"},{"line_number":58,"context_line":"        mock.patch(\u0027neutron.agent.l3.agent.L3PluginApi\u0027).start()"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"        def unique_callback_manager():"},{"line_number":61,"context_line":"            return manager.CallbacksManager()"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a6ced46_9ce1e200","line":58,"updated":"2015-03-24 15:43:36.000000000","message":"this is probably better if it was:\n\n  self.callback_manager \u003d manager.CallbacksManager()\n  mock.patch.object(registry, \u0027_get_callback_manager\u0027, return_value\u003dself.callback_manager)","commit_id":"458ca7ab3bdde6b388978c484a605cad11da0b79"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"7a45f8c2e70fdeff107cd5ecbe0f3fb896ff20cc","unresolved":false,"context_lines":[{"line_number":55,"context_line":"class L3AgentTestFramework(base.BaseOVSLinuxTestCase):"},{"line_number":56,"context_line":"    def setUp(self):"},{"line_number":57,"context_line":"        super(L3AgentTestFramework, self).setUp()"},{"line_number":58,"context_line":"        mock.patch(\u0027neutron.agent.l3.agent.L3PluginApi\u0027).start()"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"        def unique_callback_manager():"},{"line_number":61,"context_line":"            return manager.CallbacksManager()"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a6ced46_9cec8291","line":58,"in_reply_to":"1a6ced46_9ce1e200","updated":"2015-03-24 15:50:27.000000000","message":"Good idea. Will try. Thanks!","commit_id":"458ca7ab3bdde6b388978c484a605cad11da0b79"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"7a45f8c2e70fdeff107cd5ecbe0f3fb896ff20cc","unresolved":false,"context_lines":[{"line_number":59,"context_line":""},{"line_number":60,"context_line":"        def unique_callback_manager():"},{"line_number":61,"context_line":"            return manager.CallbacksManager()"},{"line_number":62,"context_line":"        mock.patch.object(registry, \u0027_get_callback_manager\u0027,"},{"line_number":63,"context_line":"                          side_effect\u003dunique_callback_manager)"},{"line_number":64,"context_line":"        self.agent \u003d self._configure_agent(\u0027agent1\u0027)"},{"line_number":65,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"1a6ced46_fcaad617","line":62,"updated":"2015-03-24 15:50:27.000000000","message":"This won\u0027t work, as _get_callback_manager is called multiple times during the test. I suspect the functional test doesn\u0027t exercise this.","commit_id":"458ca7ab3bdde6b388978c484a605cad11da0b79"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"6b8ec84f597c1c841011afd768f7d1ad689b741e","unresolved":false,"context_lines":[{"line_number":64,"context_line":""},{"line_number":65,"context_line":"        self._callback_manager \u003d manager.CallbacksManager()"},{"line_number":66,"context_line":"        mock.patch.object(registry, \u0027_get_callback_manager\u0027,"},{"line_number":67,"context_line":"                          return_value\u003dself._callback_manager).start()"},{"line_number":68,"context_line":"        self.agent \u003d self._configure_agent(\u0027agent1\u0027)"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    def _get_config_opts(self):"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa963961_08e884e1","line":67,"updated":"2015-03-27 12:14:13.000000000","message":"(No action required.) I wonder if it would make sense to have this in the base test class? You never know which test cases could end up invoking a code path that uses the registry. Something to consider when the use of callbacks increases.","commit_id":"8c5b4aa9109ee333aef219ae5b4e12fb4a4718a9"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"11bb393cc5e7972261a00a831a8c56d585c0869f","unresolved":false,"context_lines":[{"line_number":64,"context_line":""},{"line_number":65,"context_line":"        self._callback_manager \u003d manager.CallbacksManager()"},{"line_number":66,"context_line":"        mock.patch.object(registry, \u0027_get_callback_manager\u0027,"},{"line_number":67,"context_line":"                          return_value\u003dself._callback_manager).start()"},{"line_number":68,"context_line":"        self.agent \u003d self._configure_agent(\u0027agent1\u0027)"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    def _get_config_opts(self):"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa963961_0f7f63ab","line":67,"in_reply_to":"fa963961_08e884e1","updated":"2015-03-27 15:36:25.000000000","message":"Only ML2 uses the registry, so while it\u0027d do no harm performing this mock in the test base class, it has a certain odor to it :)","commit_id":"8c5b4aa9109ee333aef219ae5b4e12fb4a4718a9"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"26fb490c65f1edb739e8bba258708662a95f506c","unresolved":false,"context_lines":[{"line_number":64,"context_line":""},{"line_number":65,"context_line":"        self._callback_manager \u003d manager.CallbacksManager()"},{"line_number":66,"context_line":"        mock.patch.object(registry, \u0027_get_callback_manager\u0027,"},{"line_number":67,"context_line":"                          return_value\u003dself._callback_manager).start()"},{"line_number":68,"context_line":"        self.agent \u003d self._configure_agent(\u0027agent1\u0027)"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    def _get_config_opts(self):"}],"source_content_type":"text/x-python","patch_set":9,"id":"fa963961_92871a0f","line":67,"in_reply_to":"fa963961_0f7f63ab","updated":"2015-03-27 15:51:55.000000000","message":"Perhaps a TODO capturing what Henry said may be worth it for now. I would imagine that if we indeed find ourselves in trouble, a TODO might make it easy to find the culprit.","commit_id":"8c5b4aa9109ee333aef219ae5b4e12fb4a4718a9"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"e8af7a8e526c94547b20fb93793df7322e48e4ac","unresolved":false,"context_lines":[{"line_number":64,"context_line":""},{"line_number":65,"context_line":"        self._callback_manager \u003d manager.CallbacksManager()"},{"line_number":66,"context_line":"        mock.patch.object(registry, \u0027_get_callback_manager\u0027,"},{"line_number":67,"context_line":"                          return_value\u003dself._callback_manager).start()"},{"line_number":68,"context_line":"        self.agent \u003d self._configure_agent(\u0027agent1\u0027)"},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"    def _get_config_opts(self):"}],"source_content_type":"text/x-python","patch_set":9,"id":"da9b358b_03006e33","line":67,"in_reply_to":"fa963961_92871a0f","updated":"2015-03-30 11:30:12.000000000","message":"Done","commit_id":"8c5b4aa9109ee333aef219ae5b4e12fb4a4718a9"}],"neutron/tests/unit/test_l3_agent.py":[{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"c883ffe3a11b3ccc46b1ea61a7b4ffaf5faae81b","unresolved":false,"context_lines":[{"line_number":1469,"context_line":"        ns.delete()"},{"line_number":1470,"context_line":"        self.mock_ip.netns.delete.assert_called_once_with(\"qrouter-bar\")"},{"line_number":1471,"context_line":""},{"line_number":1472,"context_line":"    def _configure_metadata_proxy(self, enableflag\u003dTrue):"},{"line_number":1473,"context_line":"        # Note: Because the tests are starting up the agent multiple times"},{"line_number":1474,"context_line":"        # with and without a metatdata proxy configured, one of the tests"},{"line_number":1475,"context_line":"        # will end up registering the callbacks. To prevent that registration"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a80dd14_d8fe2bae","line":1472,"updated":"2015-03-17 14:48:20.000000000","message":"I think that the class setUp should call CallbacksManager.clear instead. Same deal for the L3 functional tests.\n\nI wonder how the singleton nature of the manager doesn\u0027t screw up things for the server side unit tests that use the callbacks?","commit_id":"91a2606dae06ce3e893abd9b0482ab7b88a69717"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"9b7811d56132b4716631682a817b04342d9dfff7","unresolved":false,"context_lines":[{"line_number":1469,"context_line":"        ns.delete()"},{"line_number":1470,"context_line":"        self.mock_ip.netns.delete.assert_called_once_with(\"qrouter-bar\")"},{"line_number":1471,"context_line":""},{"line_number":1472,"context_line":"    def _configure_metadata_proxy(self, enableflag\u003dTrue):"},{"line_number":1473,"context_line":"        # Note: Because the tests are starting up the agent multiple times"},{"line_number":1474,"context_line":"        # with and without a metatdata proxy configured, one of the tests"},{"line_number":1475,"context_line":"        # will end up registering the callbacks. To prevent that registration"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a80dd14_c48ba666","line":1472,"in_reply_to":"9a80dd14_d8fe2bae","updated":"2015-03-17 15:30:02.000000000","message":"OK. looks like the clear() does the same thing only cleaner.\n\nWill add to functional test as well, per IRC discussion.","commit_id":"91a2606dae06ce3e893abd9b0482ab7b88a69717"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d55a5626c927070406813c5f4cde8a86214e70af","unresolved":false,"context_lines":[{"line_number":37,"context_line":"from neutron.agent.linux import interface"},{"line_number":38,"context_line":"from neutron.agent.linux import ra"},{"line_number":39,"context_line":"from neutron.agent.metadata import driver as metadata_driver"},{"line_number":40,"context_line":"from neutron.callbacks import registry"},{"line_number":41,"context_line":"from neutron.common import config as base_config"},{"line_number":42,"context_line":"from neutron.common import constants as l3_constants"},{"line_number":43,"context_line":"from neutron.common import exceptions as n_exc"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_5d202df8","line":40,"updated":"2015-03-18 16:06:33.000000000","message":"we should mock the registry out entirely to minimize interdependencies.","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"87722984ec3a139e53c1b887f266a7e613e39a6e","unresolved":false,"context_lines":[{"line_number":37,"context_line":"from neutron.agent.linux import interface"},{"line_number":38,"context_line":"from neutron.agent.linux import ra"},{"line_number":39,"context_line":"from neutron.agent.metadata import driver as metadata_driver"},{"line_number":40,"context_line":"from neutron.callbacks import registry"},{"line_number":41,"context_line":"from neutron.common import config as base_config"},{"line_number":42,"context_line":"from neutron.common import constants as l3_constants"},{"line_number":43,"context_line":"from neutron.common import exceptions as n_exc"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_89dfc2d5","line":40,"in_reply_to":"9a80dd14_5d202df8","updated":"2015-03-18 18:54:05.000000000","message":"We need the registry so that the callbacks can be tested.","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"44bf31de6f17bf83755a890a2c90805fe111be5b","unresolved":false,"context_lines":[{"line_number":37,"context_line":"from neutron.agent.linux import interface"},{"line_number":38,"context_line":"from neutron.agent.linux import ra"},{"line_number":39,"context_line":"from neutron.agent.metadata import driver as metadata_driver"},{"line_number":40,"context_line":"from neutron.callbacks import registry"},{"line_number":41,"context_line":"from neutron.common import config as base_config"},{"line_number":42,"context_line":"from neutron.common import constants as l3_constants"},{"line_number":43,"context_line":"from neutron.common import exceptions as n_exc"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_c2e94ce4","line":40,"in_reply_to":"9a80dd14_89dfc2d5","updated":"2015-03-20 00:54:55.000000000","message":"right, I mean the use of the registry, not the module import per se!","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d55a5626c927070406813c5f4cde8a86214e70af","unresolved":false,"context_lines":[{"line_number":281,"context_line":"                          \u0027interface_driver\u0027: self.mock_driver}"},{"line_number":282,"context_line":"        # Since callbacks are managed by a singleton, clear all to prevent"},{"line_number":283,"context_line":"        # test interactions."},{"line_number":284,"context_line":"        registry.clear()"},{"line_number":285,"context_line":""},{"line_number":286,"context_line":"    def _process_router_instance_for_agent(self, agent, ri, router):"},{"line_number":287,"context_line":"        ri.router \u003d router"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_7dc7b164","line":284,"updated":"2015-03-18 16:06:33.000000000","message":"ditto","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7c531ea70b6a271366c2d7f358bddef9786e84be","unresolved":false,"context_lines":[{"line_number":281,"context_line":"                          \u0027interface_driver\u0027: self.mock_driver}"},{"line_number":282,"context_line":"        # Since callbacks are managed by a singleton, clear all to prevent"},{"line_number":283,"context_line":"        # test interactions."},{"line_number":284,"context_line":"        registry.clear()"},{"line_number":285,"context_line":""},{"line_number":286,"context_line":"    def _process_router_instance_for_agent(self, agent, ri, router):"},{"line_number":287,"context_line":"        ri.router \u003d router"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_77ea60d9","line":284,"in_reply_to":"9a80dd14_292416c5","updated":"2015-03-18 21:14:09.000000000","message":"I know perfectly well why you call clear here :)\n\nWhat I am saying is that the registry should be mocked out and we should unit test!","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"4f9c8f67b74f5a1fae462863bc8fd1e00924d872","unresolved":false,"context_lines":[{"line_number":281,"context_line":"                          \u0027interface_driver\u0027: self.mock_driver}"},{"line_number":282,"context_line":"        # Since callbacks are managed by a singleton, clear all to prevent"},{"line_number":283,"context_line":"        # test interactions."},{"line_number":284,"context_line":"        registry.clear()"},{"line_number":285,"context_line":""},{"line_number":286,"context_line":"    def _process_router_instance_for_agent(self, agent, ri, router):"},{"line_number":287,"context_line":"        ri.router \u003d router"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_fa0c23ee","line":284,"in_reply_to":"9a80dd14_77ea60d9","updated":"2015-03-20 12:18:42.000000000","message":"Will update.","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"87722984ec3a139e53c1b887f266a7e613e39a6e","unresolved":false,"context_lines":[{"line_number":281,"context_line":"                          \u0027interface_driver\u0027: self.mock_driver}"},{"line_number":282,"context_line":"        # Since callbacks are managed by a singleton, clear all to prevent"},{"line_number":283,"context_line":"        # test interactions."},{"line_number":284,"context_line":"        registry.clear()"},{"line_number":285,"context_line":""},{"line_number":286,"context_line":"    def _process_router_instance_for_agent(self, agent, ri, router):"},{"line_number":287,"context_line":"        ri.router \u003d router"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_292416c5","line":284,"in_reply_to":"9a80dd14_7dc7b164","updated":"2015-03-18 18:54:05.000000000","message":"We need to clear the callbacks registered, so that two tests won\u0027t conflict with one another (because the callback mechanism is a singleton). Initially, I was doing an unsubscribe right in the test case, but Assaf felt it was better to clear all callback subscriptions at setup in the framework, which would prevent any tests using the callback mechanism from having issues.","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"d55a5626c927070406813c5f4cde8a86214e70af","unresolved":false,"context_lines":[{"line_number":283,"context_line":"        # test interactions."},{"line_number":284,"context_line":"        registry.clear()"},{"line_number":285,"context_line":""},{"line_number":286,"context_line":"    def _process_router_instance_for_agent(self, agent, ri, router):"},{"line_number":287,"context_line":"        ri.router \u003d router"},{"line_number":288,"context_line":"        if not ri.radvd:"},{"line_number":289,"context_line":"            ri.radvd \u003d ra.DaemonMonitor(router[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_7d76d107","line":286,"updated":"2015-03-18 16:06:33.000000000","message":"do we have coverage for the callbacks at all?","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"4f9c8f67b74f5a1fae462863bc8fd1e00924d872","unresolved":false,"context_lines":[{"line_number":283,"context_line":"        # test interactions."},{"line_number":284,"context_line":"        registry.clear()"},{"line_number":285,"context_line":""},{"line_number":286,"context_line":"    def _process_router_instance_for_agent(self, agent, ri, router):"},{"line_number":287,"context_line":"        ri.router \u003d router"},{"line_number":288,"context_line":"        if not ri.radvd:"},{"line_number":289,"context_line":"            ri.radvd \u003d ra.DaemonMonitor(router[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_baa2abab","line":286,"in_reply_to":"9a80dd14_37cb8873","updated":"2015-03-20 12:18:42.000000000","message":"Not sure I follow how you\u0027d like the tests changed. However, I\u0027m hesitant to change the tests as part of this factoring effort. Is it OK to address under a separate bug?","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"87722984ec3a139e53c1b887f266a7e613e39a6e","unresolved":false,"context_lines":[{"line_number":283,"context_line":"        # test interactions."},{"line_number":284,"context_line":"        registry.clear()"},{"line_number":285,"context_line":""},{"line_number":286,"context_line":"    def _process_router_instance_for_agent(self, agent, ri, router):"},{"line_number":287,"context_line":"        ri.router \u003d router"},{"line_number":288,"context_line":"        if not ri.radvd:"},{"line_number":289,"context_line":"            ri.radvd \u003d ra.DaemonMonitor(router[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_e9e75eaa","line":286,"in_reply_to":"9a80dd14_7d76d107","updated":"2015-03-18 18:54:05.000000000","message":"Yes, in test_enable_metadata_proxy() and test_disable_metadata_proxy().","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7c531ea70b6a271366c2d7f358bddef9786e84be","unresolved":false,"context_lines":[{"line_number":283,"context_line":"        # test interactions."},{"line_number":284,"context_line":"        registry.clear()"},{"line_number":285,"context_line":""},{"line_number":286,"context_line":"    def _process_router_instance_for_agent(self, agent, ri, router):"},{"line_number":287,"context_line":"        ri.router \u003d router"},{"line_number":288,"context_line":"        if not ri.radvd:"},{"line_number":289,"context_line":"            ri.radvd \u003d ra.DaemonMonitor(router[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":4,"id":"9a80dd14_37cb8873","line":286,"in_reply_to":"9a80dd14_e9e75eaa","updated":"2015-03-18 21:14:09.000000000","message":"This should be revised IMO to test the bits in isolation.","commit_id":"5e6c43f231d970348d85cd7655f9b53f35e2739c"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"887fca963729d3298c4b46f36fcf565aeb8371b0","unresolved":false,"context_lines":[{"line_number":282,"context_line":"                          \u0027interface_driver\u0027: self.mock_driver}"},{"line_number":283,"context_line":"        # Since callbacks are managed by a singleton, clear all to prevent"},{"line_number":284,"context_line":"        # test interactions."},{"line_number":285,"context_line":"        registry.clear()"},{"line_number":286,"context_line":""},{"line_number":287,"context_line":"    def _process_router_instance_for_agent(self, agent, ri, router):"},{"line_number":288,"context_line":"        ri.router \u003d router"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a6ced46_132c2699","line":285,"updated":"2015-03-23 21:00:38.000000000","message":"This is prone to errors, and we don\u0027t want to set a bad example. I think we should be pretty prescriptive about what unit tests should do, and testing interactions is not one of them. I think we should try and mock the registry here so that we don\u0027t have potential side effects should this line got misplaced or removed. We could move it to the base test class but I\u0027d rather have less live moving parts than more. For instance, one thing I\u0027d like to work on is to switch to a greenpile to implement the notify_loop [2], and if we did that, all hell will break loose should this stay as is.\n\n [1] https://github.com/openstack/neutron/blob/master/neutron/tests/unit/ml2/test_ml2_plugin.py#L1271\n [2] https://github.com/openstack/neutron/blob/master/neutron/callbacks/manager.py#L139[2]","commit_id":"458ca7ab3bdde6b388978c484a605cad11da0b79"},{"author":{"_account_id":6659,"name":"Paul Michali","email":"pc@michali.net","username":"pcm"},"change_message_id":"650aa8845f1bc5ed2cd379617870ea1bb063c589","unresolved":false,"context_lines":[{"line_number":282,"context_line":"                          \u0027interface_driver\u0027: self.mock_driver}"},{"line_number":283,"context_line":"        # Since callbacks are managed by a singleton, clear all to prevent"},{"line_number":284,"context_line":"        # test interactions."},{"line_number":285,"context_line":"        registry.clear()"},{"line_number":286,"context_line":""},{"line_number":287,"context_line":"    def _process_router_instance_for_agent(self, agent, ri, router):"},{"line_number":288,"context_line":"        ri.router \u003d router"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a6ced46_8890ce44","line":285,"in_reply_to":"1a6ced46_132c2699","updated":"2015-03-24 11:16:01.000000000","message":"Sorry, I lost the changes I had made to use a mock. Will redo the changes...","commit_id":"458ca7ab3bdde6b388978c484a605cad11da0b79"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"9e66334932350ba061035d318fbe7fd5c2ad3389","unresolved":false,"context_lines":[{"line_number":44,"context_line":"from neutron.openstack.common import uuidutils"},{"line_number":45,"context_line":"from neutron.plugins.common import constants as p_const"},{"line_number":46,"context_line":"from neutron.tests import base"},{"line_number":47,"context_line":""},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"_uuid \u003d uuidutils.generate_uuid"},{"line_number":50,"context_line":"HOSTNAME \u003d \u0027myhost\u0027"}],"source_content_type":"text/x-python","patch_set":15,"id":"baa041b7_0e787ef1","side":"PARENT","line":47,"updated":"2015-04-02 19:04:34.000000000","message":"this is not relevant but ok.","commit_id":"7904c9c969a7869788b4d21a8ed33d7ec693d077"}]}