)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"b5f238045be5934ed7a928782427d32406ccbd00","unresolved":false,"context_lines":[{"line_number":3,"context_line":"AuthorDate: 2014-11-28 21:49:56 +0530"},{"line_number":4,"context_line":"Commit:     Numan Siddique \u003cnuman.siddique@enovance.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2014-11-30 20:39:24 +0530"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Schedule HA l3 routers only on legacy and dvr_snat agents"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch fixes the issue by checking the agent_mode values"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"5a890539_113407de","line":6,"updated":"2014-11-30 15:47:57.000000000","message":"Can you rephrase this patch and bug report so that it\u0027s clear that HA routers are not scheduled on distributed L3 agents, but that we count them while creating HA ports?","commit_id":"ad20a90b59c91a8bd7e4c070021bd8ee17974994"}],"neutron/db/l3_agentschedulers_db.py":[{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"a5b97a86ca441387a5596605d04139d5e8a9139e","unresolved":false,"context_lines":[{"line_number":366,"context_line":"                return {\u0027agents\u0027: []}"},{"line_number":367,"context_line":""},{"line_number":368,"context_line":"    def get_l3_agents(self, context, active\u003dNone, filters\u003dNone,"},{"line_number":369,"context_line":"                      ha_only\u003dFalse):"},{"line_number":370,"context_line":"        query \u003d context.session.query(agents_db.Agent)"},{"line_number":371,"context_line":"        query \u003d query.filter("},{"line_number":372,"context_line":"            agents_db.Agent.agent_type \u003d\u003d constants.AGENT_TYPE_L3)"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a890539_1f38dc9e","line":369,"updated":"2014-12-05 00:57:36.000000000","message":"why didn\u0027t you used filters instead like so:\n\nfilters\u003d{\u0027ha_only\u0027: True} or filters\u003d{\u0027ha_only\u0027: False}","commit_id":"a9c8b7a2582b0cf2116aeedfbf40f6f330af5117"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"a5b97a86ca441387a5596605d04139d5e8a9139e","unresolved":false,"context_lines":[{"line_number":378,"context_line":"                if column:"},{"line_number":379,"context_line":"                    query \u003d query.filter(column.in_(value))"},{"line_number":380,"context_line":""},{"line_number":381,"context_line":"        if ha_only:"},{"line_number":382,"context_line":"            # filter out dvr l3 agents"},{"line_number":383,"context_line":"            agent_mode_type \u003d \u0027\\\"agent_mode\\\": \\\"dvr\\\"\u0027"},{"line_number":384,"context_line":"            query \u003d query.filter(~agents_db.Agent.configurations.contains("}],"source_content_type":"text/x-python","patch_set":6,"id":"5a890539_9f4bec29","line":381,"updated":"2014-12-05 00:57:36.000000000","message":"here you could do:\n\n  if \u0027ha_only\u0027 in filters:\n    ..","commit_id":"a9c8b7a2582b0cf2116aeedfbf40f6f330af5117"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"a67bd4569a542bdf5d9c10ffdca12e9b8ee61192","unresolved":false,"context_lines":[{"line_number":376,"context_line":"                column \u003d getattr(agents_db.Agent, key, None)"},{"line_number":377,"context_line":"                if column:"},{"line_number":378,"context_line":"                    query \u003d query.filter(column.in_(value))"},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"            if filters.get(\u0027ha_only\u0027, False):"},{"line_number":381,"context_line":"                # filter out dvr l3 agents if \u0027ha_only\u0027 is True"},{"line_number":382,"context_line":"                agent_mode_type \u003d \u0027\\\"agent_mode\\\": \\\"dvr\\\"\u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a890539_0574fdfc","line":379,"updated":"2014-12-05 11:09:12.000000000","message":"I think that passing \u0027ha_only\u0027 filter is too specific to what we\u0027re trying to solve here. It doesn\u0027t match the semantics of most \u0027filter\u0027 parameters, that simply pass a filter into the DB query. I think a more generic filter would be \u0027agent_modes\u0027, an iterable of agent modes you\u0027re interested in. Default would be None of course, however if you pass in agent modes you want to match, the filter below becomes _or on every item in the collection. In this patch\u0027s context of course you\u0027d use it by passing (\u0027legacy\u0027, \u0027dvr_snat\u0027).","commit_id":"e267f032a63c61707faa2f9bd6f90f26af1fc93b"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"8f5317cd07fb7eb00821e27f18dd0fba41fe6a3e","unresolved":false,"context_lines":[{"line_number":376,"context_line":"                column \u003d getattr(agents_db.Agent, key, None)"},{"line_number":377,"context_line":"                if column:"},{"line_number":378,"context_line":"                    query \u003d query.filter(column.in_(value))"},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"            if filters.get(\u0027ha_only\u0027, False):"},{"line_number":381,"context_line":"                # filter out dvr l3 agents if \u0027ha_only\u0027 is True"},{"line_number":382,"context_line":"                agent_mode_type \u003d \u0027\\\"agent_mode\\\": \\\"dvr\\\"\u0027"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a890539_a00eb78a","line":379,"in_reply_to":"5a890539_0574fdfc","updated":"2014-12-05 11:46:45.000000000","message":"Agree with you. I will modify accordingly in the next patch","commit_id":"e267f032a63c61707faa2f9bd6f90f26af1fc93b"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"a67bd4569a542bdf5d9c10ffdca12e9b8ee61192","unresolved":false,"context_lines":[{"line_number":378,"context_line":"                    query \u003d query.filter(column.in_(value))"},{"line_number":379,"context_line":""},{"line_number":380,"context_line":"            if filters.get(\u0027ha_only\u0027, False):"},{"line_number":381,"context_line":"                # filter out dvr l3 agents if \u0027ha_only\u0027 is True"},{"line_number":382,"context_line":"                agent_mode_type \u003d \u0027\\\"agent_mode\\\": \\\"dvr\\\"\u0027"},{"line_number":383,"context_line":"                query \u003d query.filter(~agents_db.Agent.configurations.contains("},{"line_number":384,"context_line":"                    agent_mode_type))"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a890539_051dddbe","line":381,"updated":"2014-12-05 11:09:12.000000000","message":"Cool stuff!","commit_id":"e267f032a63c61707faa2f9bd6f90f26af1fc93b"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"32cccba02e07d13a050e692831e6c75a276a639b","unresolved":false,"context_lines":[{"line_number":385,"context_line":"                configuration_filter \u003d [configurations.contains(\u0027%s%s\\\"\u0027 %"},{"line_number":386,"context_line":"                                        (agent_mode_key, agent_mode))"},{"line_number":387,"context_line":"                                        for agent_mode in agent_modes]"},{"line_number":388,"context_line":"                # if legacy is present in the agent_modes filter, then"},{"line_number":389,"context_line":"                # also select agents with empty \u0027configurations\u0027"},{"line_number":390,"context_line":"                if \u0027legacy\u0027 in agent_modes:"},{"line_number":391,"context_line":"                    configuration_filter.append(configurations \u003d\u003d \u0027{}\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a890539_6e1ae270","line":388,"updated":"2014-12-06 07:34:34.000000000","message":"I am assuming that there could be legacy l3 agents with empty configurations dict.\nPlease correct me if I am wrong.","commit_id":"3f8474307bc7d71e7338b40da22d48ce40747b89"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"9e087c9353dcd9d68765e1ceacebbbff154e86e3","unresolved":false,"context_lines":[{"line_number":385,"context_line":"                configuration_filter \u003d [configurations.contains(\u0027%s%s\\\"\u0027 %"},{"line_number":386,"context_line":"                                        (agent_mode_key, agent_mode))"},{"line_number":387,"context_line":"                                        for agent_mode in agent_modes]"},{"line_number":388,"context_line":"                # if legacy is present in the agent_modes filter, then"},{"line_number":389,"context_line":"                # also select agents with empty \u0027configurations\u0027"},{"line_number":390,"context_line":"                if \u0027legacy\u0027 in agent_modes:"},{"line_number":391,"context_line":"                    configuration_filter.append(configurations \u003d\u003d \u0027{}\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a890539_eea8928e","line":388,"in_reply_to":"5a890539_6e1ae270","updated":"2014-12-06 09:26:03.000000000","message":"No the L3 agent always reports with a configurations dict:\n\nhttps://github.com/openstack/neutron/blob/master/neutron/agent/l3_agent.py#L1937","commit_id":"3f8474307bc7d71e7338b40da22d48ce40747b89"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"dbff77c97bd61febdf7fe450cecaf16295785825","unresolved":false,"context_lines":[{"line_number":379,"context_line":"                    query \u003d query.filter(column.in_(value))"},{"line_number":380,"context_line":""},{"line_number":381,"context_line":"            agent_modes \u003d filters.get(\u0027agent_modes\u0027, [])"},{"line_number":382,"context_line":"            if agent_modes:"},{"line_number":383,"context_line":"                configurations \u003d getattr(agents_db.Agent, \u0027configurations\u0027)"},{"line_number":384,"context_line":"                agent_mode_key \u003d \u0027\\\"agent_mode\\\": \\\"\u0027"},{"line_number":385,"context_line":"                configuration_filter \u003d [configurations.contains(\u0027%s%s\\\"\u0027 %"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_2c873363","line":382,"updated":"2014-12-06 17:17:14.000000000","message":"Why not just agents_db.Agent.configurations?","commit_id":"0eab99954de51827330335310a24c37b010eb77a"}],"neutron/db/l3_hamode_db.py":[{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"b5f238045be5934ed7a928782427d32406ccbd00","unresolved":false,"context_lines":[{"line_number":248,"context_line":"        0 all the agents will be used."},{"line_number":249,"context_line":"        \"\"\""},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"        min_agents \u003d cfg.CONF.min_l3_agents_per_router"},{"line_number":252,"context_line":"        num_agents \u003d len(self.get_l3_agents(context))"},{"line_number":253,"context_line":"        max_agents \u003d cfg.CONF.max_l3_agents_per_router"},{"line_number":254,"context_line":"        if max_agents:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a890539_d1227fa2","side":"PARENT","line":251,"updated":"2014-11-30 15:47:57.000000000","message":"Previously it was len(self.get_l3_agents(...)), which was reusing a method that already existed. Now, you created a new method, but the only way it\u0027s used is wrapping it in len... The new method might as well be called count_agents_for_ha_routers or similar.","commit_id":"988bd650d220b4c19b6275e4f0639e0ddceaea09"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"8cec06dcb9214c2e65b881197723fc91e1ffd043","unresolved":false,"context_lines":[{"line_number":248,"context_line":"        0 all the agents will be used."},{"line_number":249,"context_line":"        \"\"\""},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"        min_agents \u003d cfg.CONF.min_l3_agents_per_router"},{"line_number":252,"context_line":"        num_agents \u003d len(self.get_l3_agents(context))"},{"line_number":253,"context_line":"        max_agents \u003d cfg.CONF.max_l3_agents_per_router"},{"line_number":254,"context_line":"        if max_agents:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a890539_f11e9b0c","side":"PARENT","line":251,"in_reply_to":"5a890539_31e66345","updated":"2014-11-30 16:19:42.000000000","message":"Sure","commit_id":"988bd650d220b4c19b6275e4f0639e0ddceaea09"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"aa9f4009b7ff3e0824844b90b7c4fde70adbcf36","unresolved":false,"context_lines":[{"line_number":248,"context_line":"        0 all the agents will be used."},{"line_number":249,"context_line":"        \"\"\""},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"        min_agents \u003d cfg.CONF.min_l3_agents_per_router"},{"line_number":252,"context_line":"        num_agents \u003d len(self.get_l3_agents(context))"},{"line_number":253,"context_line":"        max_agents \u003d cfg.CONF.max_l3_agents_per_router"},{"line_number":254,"context_line":"        if max_agents:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a890539_31e66345","side":"PARENT","line":251,"in_reply_to":"5a890539_518a0f65","updated":"2014-11-30 16:03:03.000000000","message":"Right now we only need the count, so let\u0027s make it count. If, later, someone needs the actual agents themselves, it\u0027d be very easy to change it.","commit_id":"988bd650d220b4c19b6275e4f0639e0ddceaea09"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"d50596b0366917947ec305a3dba2af8c1311a1b1","unresolved":false,"context_lines":[{"line_number":248,"context_line":"        0 all the agents will be used."},{"line_number":249,"context_line":"        \"\"\""},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"        min_agents \u003d cfg.CONF.min_l3_agents_per_router"},{"line_number":252,"context_line":"        num_agents \u003d len(self.get_l3_agents(context))"},{"line_number":253,"context_line":"        max_agents \u003d cfg.CONF.max_l3_agents_per_router"},{"line_number":254,"context_line":"        if max_agents:"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a890539_518a0f65","side":"PARENT","line":251,"in_reply_to":"5a890539_d1227fa2","updated":"2014-11-30 16:01:29.000000000","message":"The new method \u0027get_ha_l3_agent_candidates\u0027 can be used later by other part of the code to get the l3 agent candidates as list.\nIf you think it is not required, I will change the function to return the count instead of the list.","commit_id":"988bd650d220b4c19b6275e4f0639e0ddceaea09"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"3b319d250bd70a1abdb6339b614d0225641ce9f4","unresolved":false,"context_lines":[{"line_number":240,"context_line":""},{"line_number":241,"context_line":"        return ha_network"},{"line_number":242,"context_line":""},{"line_number":243,"context_line":"    def _is_ha_candidate(self, agent):"},{"line_number":244,"context_line":"        agent_conf \u003d self.get_configuration_dict(agent)"},{"line_number":245,"context_line":"        agent_mode \u003d agent_conf.get(\u0027agent_mode\u0027, \u0027legacy\u0027)"},{"line_number":246,"context_line":"        return agent_mode in (\u0027legacy\u0027, \u0027dvr_snat\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a890539_6140f535","line":243,"updated":"2014-12-02 00:18:55.000000000","message":"have you considered modifying get_l3_agents to support a filter that allows you to select by agent_mode? This might turn to be a useful addition.","commit_id":"f5a90c16c20d12fb36be8b7a8da6b9244ba6aeca"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"3b319d250bd70a1abdb6339b614d0225641ce9f4","unresolved":false,"context_lines":[{"line_number":245,"context_line":"        agent_mode \u003d agent_conf.get(\u0027agent_mode\u0027, \u0027legacy\u0027)"},{"line_number":246,"context_line":"        return agent_mode in (\u0027legacy\u0027, \u0027dvr_snat\u0027)"},{"line_number":247,"context_line":""},{"line_number":248,"context_line":"    def get_number_of_ha_agent_candidates(self, context):"},{"line_number":249,"context_line":"        all_agents \u003d self.get_l3_agents(context)"},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"        candidates \u003d [agent for agent in all_agents"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a890539_417139a7","line":248,"updated":"2014-12-02 00:18:55.000000000","message":"I wouldn\u0027t expose this method, it\u0027s pointless unless it ran an optimized query  against the DB (for instance using the sql function count(*)), which you can\u0027t here for obvious reasons.","commit_id":"f5a90c16c20d12fb36be8b7a8da6b9244ba6aeca"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"3b319d250bd70a1abdb6339b614d0225641ce9f4","unresolved":false,"context_lines":[{"line_number":261,"context_line":"        \"\"\""},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"        min_agents \u003d cfg.CONF.min_l3_agents_per_router"},{"line_number":264,"context_line":"        num_agents \u003d self.get_number_of_ha_agent_candidates(context)"},{"line_number":265,"context_line":"        max_agents \u003d cfg.CONF.max_l3_agents_per_router"},{"line_number":266,"context_line":"        if max_agents:"},{"line_number":267,"context_line":"            if max_agents \u003e num_agents:"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a890539_61dc7575","line":264,"updated":"2014-12-02 00:18:55.000000000","message":"if you followed my suggestions above this can simply become:\n\nnum_agents \u003d len(self.get_l3_agents(context, filters\u003d{\u0027agent_mode\u0027: [\u0027legacy\u0027, \u0027dvr_snat\u0027]))","commit_id":"f5a90c16c20d12fb36be8b7a8da6b9244ba6aeca"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"797805f279c0b1cea6c75b57e460a56478347573","unresolved":false,"context_lines":[{"line_number":261,"context_line":"        \"\"\""},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"        min_agents \u003d cfg.CONF.min_l3_agents_per_router"},{"line_number":264,"context_line":"        num_agents \u003d self.get_number_of_ha_agent_candidates(context)"},{"line_number":265,"context_line":"        max_agents \u003d cfg.CONF.max_l3_agents_per_router"},{"line_number":266,"context_line":"        if max_agents:"},{"line_number":267,"context_line":"            if max_agents \u003e num_agents:"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a890539_6b752b31","line":264,"in_reply_to":"5a890539_0b5ae729","updated":"2014-12-02 06:26:40.000000000","message":"well, you can..it\u0027s not going to be a straight query to the DB, that\u0027s for sure.","commit_id":"f5a90c16c20d12fb36be8b7a8da6b9244ba6aeca"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"cdc395c4c3d96e5ab476961efe551fa1d035501f","unresolved":false,"context_lines":[{"line_number":261,"context_line":"        \"\"\""},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"        min_agents \u003d cfg.CONF.min_l3_agents_per_router"},{"line_number":264,"context_line":"        num_agents \u003d self.get_number_of_ha_agent_candidates(context)"},{"line_number":265,"context_line":"        max_agents \u003d cfg.CONF.max_l3_agents_per_router"},{"line_number":266,"context_line":"        if max_agents:"},{"line_number":267,"context_line":"            if max_agents \u003e num_agents:"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a890539_0b5ae729","line":264,"in_reply_to":"5a890539_61dc7575","updated":"2014-12-02 06:03:17.000000000","message":"I thought about it. But agent_mode is stored in the \u0027agent\u0027 table field \u0027configurations\u0027 which is a dictionary.\n\nEg.\nconfigurations \u003d {\"router_id\": \"\", \"agent_mode\": \"dvr_snat\", \"gateway_external_network_id\": \"\", \"handle_internal_only_routers\": true, \"use_namespaces\": true,....}\n\nI am not sure if i can do\nself.get_l3_agents(context, {\u0027configurations\u0027: {\u0027agent_mode: [\u0027legacy\u0027, \u0027dvr_snat\u0027]}","commit_id":"f5a90c16c20d12fb36be8b7a8da6b9244ba6aeca"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"79637c30567b0fe72b92bc400cd17f995ea408c2","unresolved":false,"context_lines":[{"line_number":261,"context_line":"        \"\"\""},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"        min_agents \u003d cfg.CONF.min_l3_agents_per_router"},{"line_number":264,"context_line":"        num_agents \u003d self.get_number_of_ha_agent_candidates(context)"},{"line_number":265,"context_line":"        max_agents \u003d cfg.CONF.max_l3_agents_per_router"},{"line_number":266,"context_line":"        if max_agents:"},{"line_number":267,"context_line":"            if max_agents \u003e num_agents:"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a890539_ab78f3f8","line":264,"in_reply_to":"5a890539_6b752b31","updated":"2014-12-02 06:37:55.000000000","message":"Thanks for the comment. I was thinking its not possible.\nI will try that.","commit_id":"f5a90c16c20d12fb36be8b7a8da6b9244ba6aeca"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"a5b97a86ca441387a5596605d04139d5e8a9139e","unresolved":false,"context_lines":[{"line_number":249,"context_line":"        \"\"\""},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"        min_agents \u003d cfg.CONF.min_l3_agents_per_router"},{"line_number":252,"context_line":"        num_agents \u003d len(self.get_l3_agents(context, ha_only\u003dTrue))"},{"line_number":253,"context_line":"        max_agents \u003d cfg.CONF.max_l3_agents_per_router"},{"line_number":254,"context_line":"        if max_agents:"},{"line_number":255,"context_line":"            if max_agents \u003e num_agents:"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a890539_1f5ffce5","line":252,"updated":"2014-12-05 00:57:36.000000000","message":"we\u0027d want a test for this.","commit_id":"a9c8b7a2582b0cf2116aeedfbf40f6f330af5117"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"5a2222e9ed5783ab40a2cae494baafed97a47da4","unresolved":false,"context_lines":[{"line_number":249,"context_line":"        \"\"\""},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"        min_agents \u003d cfg.CONF.min_l3_agents_per_router"},{"line_number":252,"context_line":"        num_agents \u003d len(self.get_l3_agents(context, ha_only\u003dTrue))"},{"line_number":253,"context_line":"        max_agents \u003d cfg.CONF.max_l3_agents_per_router"},{"line_number":254,"context_line":"        if max_agents:"},{"line_number":255,"context_line":"            if max_agents \u003e num_agents:"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a890539_f5dc11e3","line":252,"in_reply_to":"5a890539_1f5ffce5","updated":"2014-12-05 02:58:01.000000000","message":"You want a a test case of the function \"get_number_of_agents_for_scheduling\" or for \"get_l3_agents\" ?\n\nPresently in all the unit test cases I see \"get_l3_agents\" is mocked and presently there is no test case to test \"get_l3_agents\" or to test any db related functions.\n\nIf you want to test \"get_l3_agents\" I think we need to mock session and query.\nPlease correct me if i am wrong","commit_id":"a9c8b7a2582b0cf2116aeedfbf40f6f330af5117"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"5fcbfe7526e23d6fe3f8404ff6c850216fdacb31","unresolved":false,"context_lines":[{"line_number":249,"context_line":"        \"\"\""},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"        min_agents \u003d cfg.CONF.min_l3_agents_per_router"},{"line_number":252,"context_line":"        num_agents \u003d len(self.get_l3_agents(context, ha_only\u003dTrue))"},{"line_number":253,"context_line":"        max_agents \u003d cfg.CONF.max_l3_agents_per_router"},{"line_number":254,"context_line":"        if max_agents:"},{"line_number":255,"context_line":"            if max_agents \u003e num_agents:"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a890539_01b31a4e","line":252,"in_reply_to":"5a890539_56a24afd","updated":"2014-12-05 05:32:16.000000000","message":"I removed it thinking it isn\u0027t required now.\nSure. I will add the two  tests as you suggested.\nThanks","commit_id":"a9c8b7a2582b0cf2116aeedfbf40f6f330af5117"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"ec5e331c999de3efa431689f0d667db350e5ba02","unresolved":false,"context_lines":[{"line_number":249,"context_line":"        \"\"\""},{"line_number":250,"context_line":""},{"line_number":251,"context_line":"        min_agents \u003d cfg.CONF.min_l3_agents_per_router"},{"line_number":252,"context_line":"        num_agents \u003d len(self.get_l3_agents(context, ha_only\u003dTrue))"},{"line_number":253,"context_line":"        max_agents \u003d cfg.CONF.max_l3_agents_per_router"},{"line_number":254,"context_line":"        if max_agents:"},{"line_number":255,"context_line":"            if max_agents \u003e num_agents:"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a890539_56a24afd","line":252,"in_reply_to":"5a890539_f5dc11e3","updated":"2014-12-05 04:46:20.000000000","message":"You touched two methods, so ideally you\u0027d need to validate both changes; \n\nHowever I would be happy to see at least a test that validates that the issue that led you to this fix no longer occurs; you had something in the previous patchset, which you now removed. Care to elaborate why?\n\nPerhaps here it suffices that get_l3_agents is called with the expected arguments; and in agent_schedulers_db you can validate that the if statement to handle ha_only behaves as expected.","commit_id":"a9c8b7a2582b0cf2116aeedfbf40f6f330af5117"},{"author":{"_account_id":9077,"name":"Rajeev Grover","email":"rgrover687@gmail.com","username":"rajeev"},"change_message_id":"3bff9bfa8db0690112dd4160f613db6a9fdd9110","unresolved":false,"context_lines":[{"line_number":250,"context_line":""},{"line_number":251,"context_line":"        min_agents \u003d cfg.CONF.min_l3_agents_per_router"},{"line_number":252,"context_line":"        num_agents \u003d len(self.get_l3_agents(context,"},{"line_number":253,"context_line":"            filters\u003d{\u0027agent_modes\u0027: [\u0027legacy\u0027, \u0027dvr_snat\u0027]}))"},{"line_number":254,"context_line":"        max_agents \u003d cfg.CONF.max_l3_agents_per_router"},{"line_number":255,"context_line":"        if max_agents:"},{"line_number":256,"context_line":"            if max_agents \u003e num_agents:"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a961159_cb93c3f8","line":253,"updated":"2014-12-09 02:25:34.000000000","message":"The default value for agent_mod is \u0027legacy\u0027 so please make sure the case where agent_mode is not explicitly set in l3_agent.ini is also handled.","commit_id":"939a2d1a07782159db4f7b9758368b2c31979838"},{"author":{"_account_id":9077,"name":"Rajeev Grover","email":"rgrover687@gmail.com","username":"rajeev"},"change_message_id":"51d7a08a9f84906f9d2aca4eb8e44a6e1e272a4b","unresolved":false,"context_lines":[{"line_number":250,"context_line":""},{"line_number":251,"context_line":"        min_agents \u003d cfg.CONF.min_l3_agents_per_router"},{"line_number":252,"context_line":"        num_agents \u003d len(self.get_l3_agents(context,"},{"line_number":253,"context_line":"            filters\u003d{\u0027agent_modes\u0027: [\u0027legacy\u0027, \u0027dvr_snat\u0027]}))"},{"line_number":254,"context_line":"        max_agents \u003d cfg.CONF.max_l3_agents_per_router"},{"line_number":255,"context_line":"        if max_agents:"},{"line_number":256,"context_line":"            if max_agents \u003e num_agents:"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a961159_dc4e0bc1","line":253,"in_reply_to":"3a961159_39be41d5","updated":"2014-12-09 19:06:45.000000000","message":"Thanks for the explanation, Juno+ doesn\u0027t need to worry about the blank agent_mode.","commit_id":"939a2d1a07782159db4f7b9758368b2c31979838"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"5590f101c62db412489ed2b8a6429c82e67efb4c","unresolved":false,"context_lines":[{"line_number":250,"context_line":""},{"line_number":251,"context_line":"        min_agents \u003d cfg.CONF.min_l3_agents_per_router"},{"line_number":252,"context_line":"        num_agents \u003d len(self.get_l3_agents(context,"},{"line_number":253,"context_line":"            filters\u003d{\u0027agent_modes\u0027: [\u0027legacy\u0027, \u0027dvr_snat\u0027]}))"},{"line_number":254,"context_line":"        max_agents \u003d cfg.CONF.max_l3_agents_per_router"},{"line_number":255,"context_line":"        if max_agents:"},{"line_number":256,"context_line":"            if max_agents \u003e num_agents:"}],"source_content_type":"text/x-python","patch_set":11,"id":"3a961159_39be41d5","line":253,"in_reply_to":"3a961159_cb93c3f8","updated":"2014-12-09 18:31:29.000000000","message":"I\u0027m assuming you\u0027re talking about Icehouse L3 agents and older? Otherwise if a Juno+ L3 agent reports a blank agent_mode, that\u0027s just silly, we should validate input in the L3 agent so that it accepts only one of the known modes. For the upgrade case, we can set the agent_mode to legacy (If it\u0027s missing) when the controller gets a L3 status report. This way the rest of the code can assume that field is populated. It\u0027ll clean up the code a lot... There\u0027s a lot of config.get(\u0027agent_mode\u0027 ,\u0027legacy\u0027) around.\n\nAs for this patch, L3 HA is supported only in Juno+ so any L3 agent that this code runs against should only create HA ports on agents that reported an agent_mode.","commit_id":"939a2d1a07782159db4f7b9758368b2c31979838"}],"neutron/tests/unit/db/test_l3_agentschedulers_db.py":[{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"9e087c9353dcd9d68765e1ceacebbbff154e86e3","unresolved":false,"context_lines":[{"line_number":1,"context_line":"# Copyright (C) 2014 eNovance SAS \u003clicensing@enovance.com\u003e"},{"line_number":2,"context_line":"#"},{"line_number":3,"context_line":"# Licensed under the Apache License, Version 2.0 (the \"License\"); you may"},{"line_number":4,"context_line":"# not use this file except in compliance with the License. You may obtain"}],"source_content_type":"text/x-python","patch_set":9,"id":"5a890539_2e79ba30","line":1,"updated":"2014-12-06 09:26:03.000000000","message":"Why didn\u0027t you add the test to test_l3_schedulers?","commit_id":"3f8474307bc7d71e7338b40da22d48ce40747b89"}],"neutron/tests/unit/db/test_l3_ha_db.py":[{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"b5f238045be5934ed7a928782427d32406ccbd00","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        self.admin_ctx \u003d context.get_admin_context()"},{"line_number":58,"context_line":"        self.setup_coreplugin(\u0027neutron.plugins.ml2.plugin.Ml2Plugin\u0027)"},{"line_number":59,"context_line":"        self.core_plugin \u003d manager.NeutronManager.get_plugin()"},{"line_number":60,"context_line":"        mock.patch.object(l3_hamode_db.L3_HA_NAT_db_mixin, \u0027get_l3_agents\u0027,"},{"line_number":61,"context_line":"                          create\u003dTrue, return_value\u003d[_AGENT_LEGACY,"},{"line_number":62,"context_line":"                                                     _AGENT_DVR_SNAT,"},{"line_number":63,"context_line":"                                                     _AGENT_DVR]).start()"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a890539_0e06a221","line":60,"updated":"2014-11-30 15:47:57.000000000","message":"This is getting awfully fakey... Can\u0027t we generate dummy agent confs and use them here?","commit_id":"ad20a90b59c91a8bd7e4c070021bd8ee17974994"},{"author":{"_account_id":10237,"name":"Numan Siddique","email":"nusiddiq@redhat.com","username":"numansiddique"},"change_message_id":"8cec06dcb9214c2e65b881197723fc91e1ffd043","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        self.admin_ctx \u003d context.get_admin_context()"},{"line_number":58,"context_line":"        self.setup_coreplugin(\u0027neutron.plugins.ml2.plugin.Ml2Plugin\u0027)"},{"line_number":59,"context_line":"        self.core_plugin \u003d manager.NeutronManager.get_plugin()"},{"line_number":60,"context_line":"        mock.patch.object(l3_hamode_db.L3_HA_NAT_db_mixin, \u0027get_l3_agents\u0027,"},{"line_number":61,"context_line":"                          create\u003dTrue, return_value\u003d[_AGENT_LEGACY,"},{"line_number":62,"context_line":"                                                     _AGENT_DVR_SNAT,"},{"line_number":63,"context_line":"                                                     _AGENT_DVR]).start()"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a890539_5114cfe8","line":60,"in_reply_to":"5a890539_0e06a221","updated":"2014-11-30 16:19:42.000000000","message":"Agree. I will do so","commit_id":"ad20a90b59c91a8bd7e4c070021bd8ee17974994"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"651acb5d677fb18ab184010c3f600b45fb5d7298","unresolved":false,"context_lines":[{"line_number":152,"context_line":""},{"line_number":153,"context_line":"        state \u003d routers[0].get(constants.HA_ROUTER_STATE_KEY)"},{"line_number":154,"context_line":"        self.assertEqual(\u0027active\u0027, state)"},{"line_number":155,"context_line":""},{"line_number":156,"context_line":"    def test_get_number_of_ha_agent_candidates(self):"},{"line_number":157,"context_line":"        num_ha_l3_agents \u003d self.plugin.get_number_of_ha_agent_candidates("},{"line_number":158,"context_line":"            self.admin_ctx)"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a890539_f9d4b9ea","line":155,"updated":"2014-12-01 10:09:23.000000000","message":"Now with the changes I made in the dependent patch, this test could register a new DVR agent and assert that get_number_of_ha_agent_candidates returns 2 and not 3.","commit_id":"2222e0846ef02f78b4fa180b3486076da75f60ff"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"eac01e1745394f5708a9a7947cd9121139b6c15b","unresolved":false,"context_lines":[{"line_number":387,"context_line":""},{"line_number":388,"context_line":"        routers_after \u003d self.plugin.get_routers(self.admin_ctx)"},{"line_number":389,"context_line":"        self.assertEqual(routers_before, routers_after)"},{"line_number":390,"context_line":""},{"line_number":391,"context_line":"    def test_get_number_of_ha_agent_candidates(self):"},{"line_number":392,"context_line":"        dvr_agent_status \u003d {"},{"line_number":393,"context_line":"            \u0027agent_type\u0027: constants.AGENT_TYPE_L3,"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a890539_435866fa","line":390,"updated":"2014-12-01 20:14:31.000000000","message":"I think it\u0027s not trivial to understand why the test makes sense (You have to remember that the test setUp registers two agents). You can self-document it by asserting that self.get_l3_agents returns 3, and/or by adding a docstring. I\u0027m +1\u0027ing anyway, if you need to push another patchset for another reason it\u0027d be great if you could address this.","commit_id":"f5a90c16c20d12fb36be8b7a8da6b9244ba6aeca"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"3b319d250bd70a1abdb6339b614d0225641ce9f4","unresolved":false,"context_lines":[{"line_number":388,"context_line":"        routers_after \u003d self.plugin.get_routers(self.admin_ctx)"},{"line_number":389,"context_line":"        self.assertEqual(routers_before, routers_after)"},{"line_number":390,"context_line":""},{"line_number":391,"context_line":"    def test_get_number_of_ha_agent_candidates(self):"},{"line_number":392,"context_line":"        dvr_agent_status \u003d {"},{"line_number":393,"context_line":"            \u0027agent_type\u0027: constants.AGENT_TYPE_L3,"},{"line_number":394,"context_line":"            \u0027binary\u0027: \u0027neutron-l3-agent\u0027,"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a890539_21ca6da1","line":391,"updated":"2014-12-02 00:18:55.000000000","message":"I\u0027d question that even the name is pretty poor, perhaps consider adding \u0027_avoid_dvr_agents)\u0027","commit_id":"f5a90c16c20d12fb36be8b7a8da6b9244ba6aeca"},{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"a67bd4569a542bdf5d9c10ffdca12e9b8ee61192","unresolved":false,"context_lines":[{"line_number":389,"context_line":""},{"line_number":390,"context_line":"        routers_after \u003d self.plugin.get_routers(self.admin_ctx)"},{"line_number":391,"context_line":"        self.assertEqual(routers_before, routers_after)"},{"line_number":392,"context_line":""},{"line_number":393,"context_line":"    def test_exclude_dvr_agents_for_ha_candidates(self):"},{"line_number":394,"context_line":"        \"\"\"Test dvr agents are not counted in the ha candidates."},{"line_number":395,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"5a890539_254819b3","line":392,"updated":"2014-12-05 11:09:12.000000000","message":"+1","commit_id":"e267f032a63c61707faa2f9bd6f90f26af1fc93b"},{"author":{"_account_id":9077,"name":"Rajeev Grover","email":"rgrover687@gmail.com","username":"rajeev"},"change_message_id":"3bff9bfa8db0690112dd4160f613db6a9fdd9110","unresolved":false,"context_lines":[{"line_number":64,"context_line":""},{"line_number":65,"context_line":"        self.plugin.create_or_update_agent(self.admin_ctx, agent_status)"},{"line_number":66,"context_line":"        agent_status[\u0027host\u0027] \u003d \u0027l3host_2\u0027"},{"line_number":67,"context_line":"        agent_status[\u0027configurations\u0027] \u003d {\u0027agent_mode\u0027: \u0027dvr_snat\u0027}"},{"line_number":68,"context_line":"        self.plugin.create_or_update_agent(self.admin_ctx, agent_status)"},{"line_number":69,"context_line":"        self.agent1, self.agent2 \u003d self.plugin.get_agents(self.admin_ctx)"},{"line_number":70,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"3a961159_56a5b651","line":67,"updated":"2014-12-09 02:25:34.000000000","message":"You might want to include an agent with \u0027agent_mode\u0027 not set. This will cover the default case.","commit_id":"939a2d1a07782159db4f7b9758368b2c31979838"}],"neutron/tests/unit/test_l3_schedulers.py":[{"author":{"_account_id":8873,"name":"Assaf Muller","email":"amuller@redhat.com","username":"amuller"},"change_message_id":"dbff77c97bd61febdf7fe450cecaf16295785825","unresolved":false,"context_lines":[{"line_number":1344,"context_line":"        self.admin_ctx \u003d q_context.get_admin_context()"},{"line_number":1345,"context_line":"        self._register_agents()"},{"line_number":1346,"context_line":""},{"line_number":1347,"context_line":"    def _register_agents(self):"},{"line_number":1348,"context_line":"        agent_status \u003d {"},{"line_number":1349,"context_line":"            \u0027agent_type\u0027: constants.AGENT_TYPE_L3,"},{"line_number":1350,"context_line":"            \u0027binary\u0027: \u0027neutron-l3-agent\u0027,"}],"source_content_type":"text/x-python","patch_set":10,"id":"3a961159_ec728b48","line":1347,"updated":"2014-12-06 17:17:14.000000000","message":"I\u0027m thinking we refactor _register_l3_agent so that it accepts a \u0027host\u0027 and \u0027agent_mode\u0027 params. The agent_status dict will be internal to it. We could remove the FIRST_L3_AGENT, second, third and fourth dicts. Here you just call the method 5 times with different params... Reduce it from 24 to 5 lines and simplify the rest of the code as well. If you don\u0027t want to do it I\u0027d be happy to do it in the dependent patch.","commit_id":"0eab99954de51827330335310a24c37b010eb77a"}]}