)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"6dcfa3f4c5db40465e159d99c3b57bdfa21be120","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     John Davidge \u003cjohn.davidge@rackspace.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2016-07-14 14:59:39 +0100"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"[WIP] Enable CRUD for Subnet Service Types"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch enables basic CRUD operations to support"},{"line_number":10,"context_line":"Subnet service-types."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"1aa78d24_eaf3b49d","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":6},"updated":"2016-07-14 16:47:01.000000000","message":"Remove the WIP when you think it\u0027s ready for broader review.","commit_id":"dd2520b84d037d97d005319c3c2f16d6dd9a94b1"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"0443d77a35b18a421dd0a25b5d18fe28062f8a80","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch enables basic CRUD operations to support"},{"line_number":10,"context_line":"Subnet service-types."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Partially-implements: blueprint service-subnets"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Co-Authored-By: John Davidge \u003cjohn.davidge@rackspace.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"dada55a8_338a4ac0","line":11,"updated":"2016-07-19 11:33:25.000000000","message":"can you please add a release note and DocImpact here","commit_id":"b733ccd6b5619f69668563a39204eb66a2ab02a3"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"11664c1f7e82659b67d96ec77b4b1c374536a436","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This patch enables basic CRUD operations to support"},{"line_number":10,"context_line":"Subnet service-types."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Partially-implements: blueprint service-subnets"},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Co-Authored-By: John Davidge \u003cjohn.davidge@rackspace.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"dada55a8_2e213d4e","line":11,"in_reply_to":"dada55a8_338a4ac0","updated":"2016-07-19 11:49:00.000000000","message":"This patch is just laying the groundwork for a follow on patch which will actually make this feature usable. I think it would be better for the release note and DocImpact to be tied to that patch.","commit_id":"b733ccd6b5619f69668563a39204eb66a2ab02a3"},{"author":{"_account_id":22220,"name":"Anindita Das","email":"anindita.das@utexas.edu","username":"anudas"},"change_message_id":"8b2e3800e98710a121aedccf776a8edc20a74496","unresolved":false,"context_lines":[{"line_number":1,"context_line":"Parent:     38d33363 (Merge \"Update testing coverage document\")"},{"line_number":2,"context_line":"Author:     Brian Haley \u003cbrian.haley@hpe.com\u003e"},{"line_number":3,"context_line":"AuthorDate: 2016-06-23 17:48:13 -0400"},{"line_number":4,"context_line":"Commit:     Brian Haley \u003cbrian.haley@hpe.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":15,"id":"bacf61ea_a7524465","line":1,"updated":"2016-08-04 14:18:25.000000000","message":"This  change is enabling basic CRUD operations for Subnet service types. I could find create and update but no delete operation for Subnet Service Type. Do we not need the delete operation?","commit_id":"26434fbca793be3b62e8e3d94cde62b470395f19"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"9fe79461e1818f04623889f3890afd588e388b4c","unresolved":false,"context_lines":[{"line_number":1,"context_line":"Parent:     38d33363 (Merge \"Update testing coverage document\")"},{"line_number":2,"context_line":"Author:     Brian Haley \u003cbrian.haley@hpe.com\u003e"},{"line_number":3,"context_line":"AuthorDate: 2016-06-23 17:48:13 -0400"},{"line_number":4,"context_line":"Commit:     Brian Haley \u003cbrian.haley@hpe.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":15,"id":"bacf61ea_408fb674","line":1,"in_reply_to":"bacf61ea_a7524465","updated":"2016-08-05 13:46:00.000000000","message":"Deletion is handled automatically when the parent subnet is deleted. See https://review.openstack.org/#/c/337851/15/neutron/db/subnet_service_type_db_models.py","commit_id":"26434fbca793be3b62e8e3d94cde62b470395f19"}],"neutron/db/db_base_plugin_common.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"11c103751d779d343d5f7965ec51063890da9606","unresolved":false,"context_lines":[{"line_number":317,"context_line":"                args[\u0027ipv6_ra_mode\u0027] \u003d subnet[\u0027ipv6_ra_mode\u0027]"},{"line_number":318,"context_line":"            if validators.is_attr_set(subnet[\u0027ipv6_address_mode\u0027]):"},{"line_number":319,"context_line":"                args[\u0027ipv6_address_mode\u0027] \u003d subnet[\u0027ipv6_address_mode\u0027]"},{"line_number":320,"context_line":"        #if validators.is_attr_set(subnet[\u0027service_type\u0027]):"},{"line_number":321,"context_line":"        #    args[\u0027service_type\u0027] \u003d subnet[\u0027service_type\u0027]"},{"line_number":322,"context_line":"        return args"},{"line_number":323,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1aa78d24_46152647","line":320,"range":{"start_line":320,"start_character":12,"end_line":320,"end_character":59},"updated":"2016-07-06 20:35:49.000000000","message":"I think we\u0027re going to need to find a less obtrusive way to introduce these.  You might get some ideas from how I added \"segment_id\" to the subnet [1].  This seems to be very similar to that.\n\n [1] https://review.openstack.org/#/c/288774/","commit_id":"8aebc1e7ead2c17270ce301c7f35a4f67aee3042"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"af91fe95276994688f4104c0ebf39663bc96f870","unresolved":false,"context_lines":[{"line_number":317,"context_line":"                args[\u0027ipv6_ra_mode\u0027] \u003d subnet[\u0027ipv6_ra_mode\u0027]"},{"line_number":318,"context_line":"            if validators.is_attr_set(subnet[\u0027ipv6_address_mode\u0027]):"},{"line_number":319,"context_line":"                args[\u0027ipv6_address_mode\u0027] \u003d subnet[\u0027ipv6_address_mode\u0027]"},{"line_number":320,"context_line":"        #if validators.is_attr_set(subnet[\u0027service_type\u0027]):"},{"line_number":321,"context_line":"        #    args[\u0027service_type\u0027] \u003d subnet[\u0027service_type\u0027]"},{"line_number":322,"context_line":"        return args"},{"line_number":323,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1aa78d24_b80f592d","line":320,"range":{"start_line":320,"start_character":12,"end_line":320,"end_character":59},"in_reply_to":"1aa78d24_46152647","updated":"2016-07-12 10:37:28.000000000","message":"Thanks for the pointer, done.","commit_id":"8aebc1e7ead2c17270ce301c7f35a4f67aee3042"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"afec8e5058b6d207bd94703080c448b567540e73","unresolved":false,"context_lines":[{"line_number":317,"context_line":"                args[\u0027ipv6_ra_mode\u0027] \u003d subnet[\u0027ipv6_ra_mode\u0027]"},{"line_number":318,"context_line":"            if validators.is_attr_set(subnet[\u0027ipv6_address_mode\u0027]):"},{"line_number":319,"context_line":"                args[\u0027ipv6_address_mode\u0027] \u003d subnet[\u0027ipv6_address_mode\u0027]"},{"line_number":320,"context_line":"        #if validators.is_attr_set(subnet[\u0027service_type\u0027]):"},{"line_number":321,"context_line":"        #    args[\u0027service_type\u0027] \u003d subnet[\u0027service_type\u0027]"},{"line_number":322,"context_line":"        return args"},{"line_number":323,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1aa78d24_838d00d5","line":320,"range":{"start_line":320,"start_character":12,"end_line":320,"end_character":59},"in_reply_to":"1aa78d24_46152647","updated":"2016-07-07 18:08:07.000000000","message":"Yes, this was cruft left-over from an earlier version","commit_id":"8aebc1e7ead2c17270ce301c7f35a4f67aee3042"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"6dcfa3f4c5db40465e159d99c3b57bdfa21be120","unresolved":false,"context_lines":[{"line_number":148,"context_line":"               \u0027host_routes\u0027: [{\u0027destination\u0027: route[\u0027destination\u0027],"},{"line_number":149,"context_line":"                                \u0027nexthop\u0027: route[\u0027nexthop\u0027]}"},{"line_number":150,"context_line":"                               for route in subnet[\u0027routes\u0027]],"},{"line_number":151,"context_line":"               \u0027service_type\u0027: [service_type[\u0027service_type\u0027] for service_type"},{"line_number":152,"context_line":"                                in subnet[\u0027service_type\u0027]]"},{"line_number":153,"context_line":"               }"},{"line_number":154,"context_line":"        # The shared attribute for a subnet is the same as its parent network"}],"source_content_type":"text/x-python","patch_set":5,"id":"1aa78d24_8a5e7077","line":151,"range":{"start_line":151,"start_character":16,"end_line":151,"end_character":28},"updated":"2016-07-14 16:47:01.000000000","message":"This should be with a register_dict_extend_funcs method and shouldn\u0027t be in this file.  In fact, this file shouldn\u0027t change at all.","commit_id":"dd2520b84d037d97d005319c3c2f16d6dd9a94b1"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"61158d387282c13a9182365736d57743e53c3c42","unresolved":false,"context_lines":[{"line_number":148,"context_line":"               \u0027host_routes\u0027: [{\u0027destination\u0027: route[\u0027destination\u0027],"},{"line_number":149,"context_line":"                                \u0027nexthop\u0027: route[\u0027nexthop\u0027]}"},{"line_number":150,"context_line":"                               for route in subnet[\u0027routes\u0027]],"},{"line_number":151,"context_line":"               \u0027service_type\u0027: [service_type[\u0027service_type\u0027] for service_type"},{"line_number":152,"context_line":"                                in subnet[\u0027service_type\u0027]]"},{"line_number":153,"context_line":"               }"},{"line_number":154,"context_line":"        # The shared attribute for a subnet is the same as its parent network"}],"source_content_type":"text/x-python","patch_set":5,"id":"fad55976_904edc94","line":151,"range":{"start_line":151,"start_character":16,"end_line":151,"end_character":28},"in_reply_to":"1aa78d24_8a5e7077","updated":"2016-07-15 15:50:02.000000000","message":"Done","commit_id":"dd2520b84d037d97d005319c3c2f16d6dd9a94b1"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"6dcfa3f4c5db40465e159d99c3b57bdfa21be120","unresolved":false,"context_lines":[{"line_number":317,"context_line":"                args[\u0027ipv6_ra_mode\u0027] \u003d subnet[\u0027ipv6_ra_mode\u0027]"},{"line_number":318,"context_line":"            if validators.is_attr_set(subnet[\u0027ipv6_address_mode\u0027]):"},{"line_number":319,"context_line":"                args[\u0027ipv6_address_mode\u0027] \u003d subnet[\u0027ipv6_address_mode\u0027]"},{"line_number":320,"context_line":"        if validators.is_attr_set(subnet.get(\u0027service_type\u0027)):"},{"line_number":321,"context_line":"            args[\u0027service_type\u0027] \u003d subnet[\u0027service_type\u0027]"},{"line_number":322,"context_line":"        return args"},{"line_number":323,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"1aa78d24_cae6b8af","line":320,"range":{"start_line":320,"start_character":11,"end_line":320,"end_character":21},"updated":"2016-07-14 16:47:01.000000000","message":"I wonder how we could avoid adding this to db_base_plugin.  Actually, you\u0027ve added this to ipam_backend_mixin, too.  I think this one is redundant.","commit_id":"dd2520b84d037d97d005319c3c2f16d6dd9a94b1"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"61158d387282c13a9182365736d57743e53c3c42","unresolved":false,"context_lines":[{"line_number":317,"context_line":"                args[\u0027ipv6_ra_mode\u0027] \u003d subnet[\u0027ipv6_ra_mode\u0027]"},{"line_number":318,"context_line":"            if validators.is_attr_set(subnet[\u0027ipv6_address_mode\u0027]):"},{"line_number":319,"context_line":"                args[\u0027ipv6_address_mode\u0027] \u003d subnet[\u0027ipv6_address_mode\u0027]"},{"line_number":320,"context_line":"        if validators.is_attr_set(subnet.get(\u0027service_type\u0027)):"},{"line_number":321,"context_line":"            args[\u0027service_type\u0027] \u003d subnet[\u0027service_type\u0027]"},{"line_number":322,"context_line":"        return args"},{"line_number":323,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"fad55976_d048548a","line":320,"range":{"start_line":320,"start_character":11,"end_line":320,"end_character":21},"in_reply_to":"1aa78d24_cae6b8af","updated":"2016-07-15 15:50:02.000000000","message":"Done","commit_id":"dd2520b84d037d97d005319c3c2f16d6dd9a94b1"}],"neutron/db/db_base_plugin_v2.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"11c103751d779d343d5f7965ec51063890da9606","unresolved":false,"context_lines":[{"line_number":686,"context_line":"        prefixlen \u003d s.get(\u0027prefixlen\u0027, constants.ATTR_NOT_SPECIFIED)"},{"line_number":687,"context_line":"        has_cidr \u003d validators.is_attr_set(cidr)"},{"line_number":688,"context_line":"        has_prefixlen \u003d validators.is_attr_set(prefixlen)"},{"line_number":689,"context_line":"        #servicetype \u003d s.get(\u0027service_type\u0027, constants.ATTR_NOT_SPECIFIED)"},{"line_number":690,"context_line":"        #has_servicetype \u003d validators.is_attr_set(servicetype)"},{"line_number":691,"context_line":""},{"line_number":692,"context_line":"        if has_cidr and has_prefixlen:"},{"line_number":693,"context_line":"            msg \u003d _(\u0027cidr and prefixlen must not be supplied together\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1aa78d24_c6011608","line":690,"range":{"start_line":689,"start_character":8,"end_line":690,"end_character":62},"updated":"2016-07-06 20:35:49.000000000","message":"ditto","commit_id":"8aebc1e7ead2c17270ce301c7f35a4f67aee3042"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"af91fe95276994688f4104c0ebf39663bc96f870","unresolved":false,"context_lines":[{"line_number":686,"context_line":"        prefixlen \u003d s.get(\u0027prefixlen\u0027, constants.ATTR_NOT_SPECIFIED)"},{"line_number":687,"context_line":"        has_cidr \u003d validators.is_attr_set(cidr)"},{"line_number":688,"context_line":"        has_prefixlen \u003d validators.is_attr_set(prefixlen)"},{"line_number":689,"context_line":"        #servicetype \u003d s.get(\u0027service_type\u0027, constants.ATTR_NOT_SPECIFIED)"},{"line_number":690,"context_line":"        #has_servicetype \u003d validators.is_attr_set(servicetype)"},{"line_number":691,"context_line":""},{"line_number":692,"context_line":"        if has_cidr and has_prefixlen:"},{"line_number":693,"context_line":"            msg \u003d _(\u0027cidr and prefixlen must not be supplied together\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"1aa78d24_582595ac","line":690,"range":{"start_line":689,"start_character":8,"end_line":690,"end_character":62},"in_reply_to":"1aa78d24_c6011608","updated":"2016-07-12 10:37:28.000000000","message":"Done","commit_id":"8aebc1e7ead2c17270ce301c7f35a4f67aee3042"}],"neutron/db/ipam_backend_mixin.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"6dcfa3f4c5db40465e159d99c3b57bdfa21be120","unresolved":false,"context_lines":[{"line_number":464,"context_line":"                                           subnet_args[\u0027subnetpool_id\u0027],"},{"line_number":465,"context_line":"                                           subnet_args[\u0027ip_version\u0027])"},{"line_number":466,"context_line":""},{"line_number":467,"context_line":"        service_types \u003d []"},{"line_number":468,"context_line":"        if subnet_args.get(\u0027service_type\u0027):"},{"line_number":469,"context_line":"            service_types \u003d subnet_args[\u0027service_type\u0027]"},{"line_number":470,"context_line":"            subnet_args[\u0027service_type\u0027] \u003d []"},{"line_number":471,"context_line":""},{"line_number":472,"context_line":"        subnet \u003d models_v2.Subnet(**subnet_args)"},{"line_number":473,"context_line":"        segment_id \u003d subnet_args.get(\u0027segment_id\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"1aa78d24_4ddae218","line":470,"range":{"start_line":467,"start_character":8,"end_line":470,"end_character":44},"updated":"2016-07-14 16:47:01.000000000","message":"This is essentially this, I think:\n\n  service_types \u003d subnet_args.pop(\u0027service_type\u0027, [])\n\nExcept your version leaves an empty list in the dict for \u0027service_type\u0027.  Is that what you really want?  I think pop might be more appropriate.","commit_id":"dd2520b84d037d97d005319c3c2f16d6dd9a94b1"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"61158d387282c13a9182365736d57743e53c3c42","unresolved":false,"context_lines":[{"line_number":464,"context_line":"                                           subnet_args[\u0027subnetpool_id\u0027],"},{"line_number":465,"context_line":"                                           subnet_args[\u0027ip_version\u0027])"},{"line_number":466,"context_line":""},{"line_number":467,"context_line":"        service_types \u003d []"},{"line_number":468,"context_line":"        if subnet_args.get(\u0027service_type\u0027):"},{"line_number":469,"context_line":"            service_types \u003d subnet_args[\u0027service_type\u0027]"},{"line_number":470,"context_line":"            subnet_args[\u0027service_type\u0027] \u003d []"},{"line_number":471,"context_line":""},{"line_number":472,"context_line":"        subnet \u003d models_v2.Subnet(**subnet_args)"},{"line_number":473,"context_line":"        segment_id \u003d subnet_args.get(\u0027segment_id\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"fad55976_730372b9","line":470,"range":{"start_line":467,"start_character":8,"end_line":470,"end_character":44},"in_reply_to":"1aa78d24_4ddae218","updated":"2016-07-15 15:50:02.000000000","message":"Yes, this is much cleaner, thanks.","commit_id":"dd2520b84d037d97d005319c3c2f16d6dd9a94b1"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"6dcfa3f4c5db40465e159d99c3b57bdfa21be120","unresolved":false,"context_lines":[{"line_number":496,"context_line":"                    nexthop\u003drt[\u0027nexthop\u0027])"},{"line_number":497,"context_line":"                context.session.add(route)"},{"line_number":498,"context_line":""},{"line_number":499,"context_line":"        if service_types:"},{"line_number":500,"context_line":"            for service_type in service_types:"},{"line_number":501,"context_line":"                service_type_entry \u003d service_type_db.SubnetServiceType("},{"line_number":502,"context_line":"                    tenant_id\u003dsubnet.tenant_id,"},{"line_number":503,"context_line":"                    subnet_id\u003dsubnet.id,"},{"line_number":504,"context_line":"                    subnet\u003dsubnet,"},{"line_number":505,"context_line":"                    service_type\u003dservice_type)"},{"line_number":506,"context_line":"                context.session.add(service_type_entry)"},{"line_number":507,"context_line":""},{"line_number":508,"context_line":"        self.save_allocation_pools(context, subnet,"},{"line_number":509,"context_line":"                                   subnet_request.allocation_pools)"}],"source_content_type":"text/x-python","patch_set":5,"id":"1aa78d24_6d635e42","line":506,"range":{"start_line":499,"start_character":8,"end_line":506,"end_character":55},"updated":"2016-07-14 16:47:01.000000000","message":"I don\u0027t think this handles update correctly.  Don\u0027t you need to consider the service types already in the DB and either avoid trying to re-insert the ones that exist and delete the ones that should no longer exist?","commit_id":"dd2520b84d037d97d005319c3c2f16d6dd9a94b1"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"61158d387282c13a9182365736d57743e53c3c42","unresolved":false,"context_lines":[{"line_number":496,"context_line":"                    nexthop\u003drt[\u0027nexthop\u0027])"},{"line_number":497,"context_line":"                context.session.add(route)"},{"line_number":498,"context_line":""},{"line_number":499,"context_line":"        if service_types:"},{"line_number":500,"context_line":"            for service_type in service_types:"},{"line_number":501,"context_line":"                service_type_entry \u003d service_type_db.SubnetServiceType("},{"line_number":502,"context_line":"                    tenant_id\u003dsubnet.tenant_id,"},{"line_number":503,"context_line":"                    subnet_id\u003dsubnet.id,"},{"line_number":504,"context_line":"                    subnet\u003dsubnet,"},{"line_number":505,"context_line":"                    service_type\u003dservice_type)"},{"line_number":506,"context_line":"                context.session.add(service_type_entry)"},{"line_number":507,"context_line":""},{"line_number":508,"context_line":"        self.save_allocation_pools(context, subnet,"},{"line_number":509,"context_line":"                                   subnet_request.allocation_pools)"}],"source_content_type":"text/x-python","patch_set":5,"id":"fad55976_e42af62b","line":506,"range":{"start_line":499,"start_character":8,"end_line":506,"end_character":55},"in_reply_to":"1aa78d24_6d635e42","updated":"2016-07-15 15:50:02.000000000","message":"This actually raises a very good question - how do we want to handle updates to the service_type list? Do we require that the entire list is passed in the subnet update request, or do we allow passing only new list items to be added - in which case how would we handle deletions? It seems to me that the only way to allow deletions with the current framework would be to require the user to pass the complete list in every update request, but that doesn\u0027t sound ideal for the user. Carl, Brian, do you have any thoughts on this?","commit_id":"dd2520b84d037d97d005319c3c2f16d6dd9a94b1"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"2bbb9eae1476da85cb51a5aded88209cca1d27cd","unresolved":false,"context_lines":[{"line_number":496,"context_line":"                    nexthop\u003drt[\u0027nexthop\u0027])"},{"line_number":497,"context_line":"                context.session.add(route)"},{"line_number":498,"context_line":""},{"line_number":499,"context_line":"        if service_types:"},{"line_number":500,"context_line":"            for service_type in service_types:"},{"line_number":501,"context_line":"                service_type_entry \u003d service_type_db.SubnetServiceType("},{"line_number":502,"context_line":"                    tenant_id\u003dsubnet.tenant_id,"},{"line_number":503,"context_line":"                    subnet_id\u003dsubnet.id,"},{"line_number":504,"context_line":"                    subnet\u003dsubnet,"},{"line_number":505,"context_line":"                    service_type\u003dservice_type)"},{"line_number":506,"context_line":"                context.session.add(service_type_entry)"},{"line_number":507,"context_line":""},{"line_number":508,"context_line":"        self.save_allocation_pools(context, subnet,"},{"line_number":509,"context_line":"                                   subnet_request.allocation_pools)"}],"source_content_type":"text/x-python","patch_set":5,"id":"dada55a8_30085c91","line":506,"range":{"start_line":499,"start_character":8,"end_line":506,"end_character":55},"in_reply_to":"dada55a8_19e9601b","updated":"2016-07-19 22:21:10.000000000","message":"Yes, we should require the entire list to be passed each time.","commit_id":"dd2520b84d037d97d005319c3c2f16d6dd9a94b1"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"134cf0d5b5dde66c616e37edb78efc5a9e8ff3f4","unresolved":false,"context_lines":[{"line_number":496,"context_line":"                    nexthop\u003drt[\u0027nexthop\u0027])"},{"line_number":497,"context_line":"                context.session.add(route)"},{"line_number":498,"context_line":""},{"line_number":499,"context_line":"        if service_types:"},{"line_number":500,"context_line":"            for service_type in service_types:"},{"line_number":501,"context_line":"                service_type_entry \u003d service_type_db.SubnetServiceType("},{"line_number":502,"context_line":"                    tenant_id\u003dsubnet.tenant_id,"},{"line_number":503,"context_line":"                    subnet_id\u003dsubnet.id,"},{"line_number":504,"context_line":"                    subnet\u003dsubnet,"},{"line_number":505,"context_line":"                    service_type\u003dservice_type)"},{"line_number":506,"context_line":"                context.session.add(service_type_entry)"},{"line_number":507,"context_line":""},{"line_number":508,"context_line":"        self.save_allocation_pools(context, subnet,"},{"line_number":509,"context_line":"                                   subnet_request.allocation_pools)"}],"source_content_type":"text/x-python","patch_set":5,"id":"fad55976_e745d517","line":506,"range":{"start_line":499,"start_character":8,"end_line":506,"end_character":55},"in_reply_to":"fad55976_e42af62b","updated":"2016-07-15 17:11:36.000000000","message":"I think this should work the way dns_nameservers does, for that you can \"unset\" a single value (at least that\u0027s what the OSC calls it).  Let me push that patch so you can see what I mean.","commit_id":"dd2520b84d037d97d005319c3c2f16d6dd9a94b1"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"975247192a2ead01d8db106cdcaa61de037810f6","unresolved":false,"context_lines":[{"line_number":496,"context_line":"                    nexthop\u003drt[\u0027nexthop\u0027])"},{"line_number":497,"context_line":"                context.session.add(route)"},{"line_number":498,"context_line":""},{"line_number":499,"context_line":"        if service_types:"},{"line_number":500,"context_line":"            for service_type in service_types:"},{"line_number":501,"context_line":"                service_type_entry \u003d service_type_db.SubnetServiceType("},{"line_number":502,"context_line":"                    tenant_id\u003dsubnet.tenant_id,"},{"line_number":503,"context_line":"                    subnet_id\u003dsubnet.id,"},{"line_number":504,"context_line":"                    subnet\u003dsubnet,"},{"line_number":505,"context_line":"                    service_type\u003dservice_type)"},{"line_number":506,"context_line":"                context.session.add(service_type_entry)"},{"line_number":507,"context_line":""},{"line_number":508,"context_line":"        self.save_allocation_pools(context, subnet,"},{"line_number":509,"context_line":"                                   subnet_request.allocation_pools)"}],"source_content_type":"text/x-python","patch_set":5,"id":"dada55a8_19e9601b","line":506,"range":{"start_line":499,"start_character":8,"end_line":506,"end_character":55},"in_reply_to":"fad55976_e745d517","updated":"2016-07-18 12:01:32.000000000","message":"Ok, so reading through the UnsetSubnet class in OSC it looks like we\u0027d be requiring the complete list to be provided in each subnet update call. I\u0027ll go with that approach then.","commit_id":"dd2520b84d037d97d005319c3c2f16d6dd9a94b1"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"41116dce9d1a616ed718cf24f38c4ed95912c8e0","unresolved":false,"context_lines":[{"line_number":512,"context_line":"                    nexthop\u003drt[\u0027nexthop\u0027])"},{"line_number":513,"context_line":"                context.session.add(route)"},{"line_number":514,"context_line":""},{"line_number":515,"context_line":"        if service_types:"},{"line_number":516,"context_line":"            for service_type in service_types:"},{"line_number":517,"context_line":"                service_type_entry \u003d service_type_db.SubnetServiceType("},{"line_number":518,"context_line":"                    tenant_id\u003dsubnet.tenant_id,"}],"source_content_type":"text/x-python","patch_set":6,"id":"dada55a8_c2ee7a8a","line":515,"updated":"2016-07-19 10:23:31.000000000","message":"I think that this should be:\nif validators.is_attr_set(service_types):","commit_id":"2c36c20e4c818b660176efba810cbf438ba1e879"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"383ba0d5a90f23826816ad27399f0e434cde0d87","unresolved":false,"context_lines":[{"line_number":512,"context_line":"                    nexthop\u003drt[\u0027nexthop\u0027])"},{"line_number":513,"context_line":"                context.session.add(route)"},{"line_number":514,"context_line":""},{"line_number":515,"context_line":"        if service_types:"},{"line_number":516,"context_line":"            for service_type in service_types:"},{"line_number":517,"context_line":"                service_type_entry \u003d service_type_db.SubnetServiceType("},{"line_number":518,"context_line":"                    tenant_id\u003dsubnet.tenant_id,"}],"source_content_type":"text/x-python","patch_set":6,"id":"dada55a8_252160b5","line":515,"in_reply_to":"dada55a8_c2ee7a8a","updated":"2016-07-19 11:14:54.000000000","message":"Done","commit_id":"2c36c20e4c818b660176efba810cbf438ba1e879"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"e86e07d5d9880d3fed1a60e618a054c0dad7e45b","unresolved":false,"context_lines":[{"line_number":182,"context_line":"        context.session.query(service_type_db.SubnetServiceType).filter_by("},{"line_number":183,"context_line":"                                 subnet_id\u003dsubnet_id).delete()"},{"line_number":184,"context_line":"        subnet \u003d self._get_subnet(context, subnet_id)"},{"line_number":185,"context_line":"        updated_types \u003d s[\u0027service_type\u0027]"},{"line_number":186,"context_line":"        new_types \u003d [service_type_db.SubnetServiceType("},{"line_number":187,"context_line":"                         subnet_id\u003dsubnet_id,"},{"line_number":188,"context_line":"                         subnet\u003dsubnet,"}],"source_content_type":"text/x-python","patch_set":7,"id":"dada55a8_30d7bc24","line":185,"range":{"start_line":185,"start_character":8,"end_line":185,"end_character":41},"updated":"2016-07-19 23:05:49.000000000","message":"nit:  How about this:\n\n  update_types s.pop(\u0027service_type\u0027)","commit_id":"b733ccd6b5619f69668563a39204eb66a2ab02a3"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"9454edf998abcd34a9bd5890b9af11c3acdecf61","unresolved":false,"context_lines":[{"line_number":182,"context_line":"        context.session.query(service_type_db.SubnetServiceType).filter_by("},{"line_number":183,"context_line":"                                 subnet_id\u003dsubnet_id).delete()"},{"line_number":184,"context_line":"        subnet \u003d self._get_subnet(context, subnet_id)"},{"line_number":185,"context_line":"        updated_types \u003d s[\u0027service_type\u0027]"},{"line_number":186,"context_line":"        new_types \u003d [service_type_db.SubnetServiceType("},{"line_number":187,"context_line":"                         subnet_id\u003dsubnet_id,"},{"line_number":188,"context_line":"                         subnet\u003dsubnet,"}],"source_content_type":"text/x-python","patch_set":7,"id":"dada55a8_87df340c","line":185,"range":{"start_line":185,"start_character":8,"end_line":185,"end_character":41},"in_reply_to":"dada55a8_30d7bc24","updated":"2016-07-20 11:43:19.000000000","message":"Done","commit_id":"b733ccd6b5619f69668563a39204eb66a2ab02a3"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"e86e07d5d9880d3fed1a60e618a054c0dad7e45b","unresolved":false,"context_lines":[{"line_number":183,"context_line":"                                 subnet_id\u003dsubnet_id).delete()"},{"line_number":184,"context_line":"        subnet \u003d self._get_subnet(context, subnet_id)"},{"line_number":185,"context_line":"        updated_types \u003d s[\u0027service_type\u0027]"},{"line_number":186,"context_line":"        new_types \u003d [service_type_db.SubnetServiceType("},{"line_number":187,"context_line":"                         subnet_id\u003dsubnet_id,"},{"line_number":188,"context_line":"                         subnet\u003dsubnet,"},{"line_number":189,"context_line":"                         service_type\u003dservice_type)"}],"source_content_type":"text/x-python","patch_set":7,"id":"dada55a8_f0a6c451","line":186,"updated":"2016-07-19 23:05:49.000000000","message":"nit:  Why not do a straight for-loop (except to follow the pattern of the preceding method). It would avoid the creation of an intermediate list.\n\n  updated_types \u003d s.pop(\u0027service_type\u0027)\n  for service_type in updated_types:\n      new_type \u003d service_type_db.SubnetServiceType(...)\n      context.session.add(new_type)\n  return updated_types","commit_id":"b733ccd6b5619f69668563a39204eb66a2ab02a3"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"9454edf998abcd34a9bd5890b9af11c3acdecf61","unresolved":false,"context_lines":[{"line_number":183,"context_line":"                                 subnet_id\u003dsubnet_id).delete()"},{"line_number":184,"context_line":"        subnet \u003d self._get_subnet(context, subnet_id)"},{"line_number":185,"context_line":"        updated_types \u003d s[\u0027service_type\u0027]"},{"line_number":186,"context_line":"        new_types \u003d [service_type_db.SubnetServiceType("},{"line_number":187,"context_line":"                         subnet_id\u003dsubnet_id,"},{"line_number":188,"context_line":"                         subnet\u003dsubnet,"},{"line_number":189,"context_line":"                         service_type\u003dservice_type)"}],"source_content_type":"text/x-python","patch_set":7,"id":"dada55a8_e7539069","line":186,"in_reply_to":"dada55a8_f0a6c451","updated":"2016-07-20 11:43:19.000000000","message":"Done","commit_id":"b733ccd6b5619f69668563a39204eb66a2ab02a3"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"e05604820ae7407ead23f1de2581147b6db6a4fc","unresolved":false,"context_lines":[{"line_number":186,"context_line":"        for service_type in updated_types:"},{"line_number":187,"context_line":"            new_type \u003d service_type_db.SubnetServiceType("},{"line_number":188,"context_line":"                           subnet_id\u003dsubnet_id,"},{"line_number":189,"context_line":"                           subnet\u003dsubnet,"},{"line_number":190,"context_line":"                           service_type\u003dservice_type)"},{"line_number":191,"context_line":"            context.session.add(new_type)"},{"line_number":192,"context_line":"        return updated_types"}],"source_content_type":"text/x-python","patch_set":10,"id":"dada55a8_07fb127a","line":189,"range":{"start_line":189,"start_character":27,"end_line":189,"end_character":40},"updated":"2016-07-27 04:23:36.000000000","message":"It just occurred to me that I don\u0027t think you should have to pass this. This is an automatic relationship which is computed from the subnet_id. This means you probably won\u0027t have to have L184 at all.\n\nIOW, remove L184 and L189.","commit_id":"8667642b320eb0f54b84d70cd298dca5a7d4841c"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"59ff64ef496da3fc3a18ad44880eec78dc5b67f6","unresolved":false,"context_lines":[{"line_number":186,"context_line":"        for service_type in updated_types:"},{"line_number":187,"context_line":"            new_type \u003d service_type_db.SubnetServiceType("},{"line_number":188,"context_line":"                           subnet_id\u003dsubnet_id,"},{"line_number":189,"context_line":"                           subnet\u003dsubnet,"},{"line_number":190,"context_line":"                           service_type\u003dservice_type)"},{"line_number":191,"context_line":"            context.session.add(new_type)"},{"line_number":192,"context_line":"        return updated_types"}],"source_content_type":"text/x-python","patch_set":10,"id":"dada55a8_586836f5","line":189,"range":{"start_line":189,"start_character":27,"end_line":189,"end_character":40},"in_reply_to":"dada55a8_07fb127a","updated":"2016-07-27 13:42:58.000000000","message":"Yep, you\u0027re quite right. Thanks, done.","commit_id":"8667642b320eb0f54b84d70cd298dca5a7d4841c"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"e05604820ae7407ead23f1de2581147b6db6a4fc","unresolved":false,"context_lines":[{"line_number":523,"context_line":"            for service_type in service_types:"},{"line_number":524,"context_line":"                service_type_entry \u003d service_type_db.SubnetServiceType("},{"line_number":525,"context_line":"                    subnet_id\u003dsubnet.id,"},{"line_number":526,"context_line":"                    subnet\u003dsubnet,"},{"line_number":527,"context_line":"                    service_type\u003dservice_type)"},{"line_number":528,"context_line":"                context.session.add(service_type_entry)"},{"line_number":529,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"dada55a8_e76a3e19","line":526,"range":{"start_line":526,"start_character":20,"end_line":526,"end_character":33},"updated":"2016-07-27 04:23:36.000000000","message":"ditto","commit_id":"8667642b320eb0f54b84d70cd298dca5a7d4841c"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"59ff64ef496da3fc3a18ad44880eec78dc5b67f6","unresolved":false,"context_lines":[{"line_number":523,"context_line":"            for service_type in service_types:"},{"line_number":524,"context_line":"                service_type_entry \u003d service_type_db.SubnetServiceType("},{"line_number":525,"context_line":"                    subnet_id\u003dsubnet.id,"},{"line_number":526,"context_line":"                    subnet\u003dsubnet,"},{"line_number":527,"context_line":"                    service_type\u003dservice_type)"},{"line_number":528,"context_line":"                context.session.add(service_type_entry)"},{"line_number":529,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"dada55a8_3bf1fc95","line":526,"range":{"start_line":526,"start_character":20,"end_line":526,"end_character":33},"in_reply_to":"dada55a8_e76a3e19","updated":"2016-07-27 13:42:58.000000000","message":"Done","commit_id":"8667642b320eb0f54b84d70cd298dca5a7d4841c"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"671f32363a902788f3ce09b53f927d12e85a3cc9","unresolved":false,"context_lines":[{"line_number":180,"context_line":""},{"line_number":181,"context_line":"    def _update_subnet_service_type(self, context, subnet_id, s):"},{"line_number":182,"context_line":"        context.session.query(service_type_db.SubnetServiceType).filter_by("},{"line_number":183,"context_line":"                                 subnet_id\u003dsubnet_id).delete()"},{"line_number":184,"context_line":"        updated_types \u003d s.pop(\u0027service_type\u0027)"},{"line_number":185,"context_line":"        for service_type in updated_types:"},{"line_number":186,"context_line":"            new_type \u003d service_type_db.SubnetServiceType("}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_cddd3d72","line":183,"updated":"2016-07-28 04:16:47.000000000","message":"please change this to actually retrieve the object and then use context.session.delete() to remove it. I know this pattern exists already in the file, but we need to get rid of it.\n\nIssuing a .delete() statement from a query directly precludes us from using the DB event handler to detect when things are removed.\n\nFor example, we will likely want this to bump the revision when the service type changes, and if it\u0027s just clearing with .delete() the revision handler won\u0027t notice the change and won\u0027t bump the revision unless new types are added.","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"233ef0644b07089f058ef6db58a79970cfb20dbc","unresolved":false,"context_lines":[{"line_number":180,"context_line":""},{"line_number":181,"context_line":"    def _update_subnet_service_type(self, context, subnet_id, s):"},{"line_number":182,"context_line":"        context.session.query(service_type_db.SubnetServiceType).filter_by("},{"line_number":183,"context_line":"                                 subnet_id\u003dsubnet_id).delete()"},{"line_number":184,"context_line":"        updated_types \u003d s.pop(\u0027service_type\u0027)"},{"line_number":185,"context_line":"        for service_type in updated_types:"},{"line_number":186,"context_line":"            new_type \u003d service_type_db.SubnetServiceType("}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_219679f0","line":183,"in_reply_to":"bacf61ea_cddd3d72","updated":"2016-07-28 18:24:43.000000000","message":"Quota layer needs this too, though in this case no quota is affected.","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"dce97d6e567d77f9fabf2cae3ce45fd4aed08ca1","unresolved":false,"context_lines":[{"line_number":180,"context_line":""},{"line_number":181,"context_line":"    def _update_subnet_service_type(self, context, subnet_id, s):"},{"line_number":182,"context_line":"        context.session.query(service_type_db.SubnetServiceType).filter_by("},{"line_number":183,"context_line":"                                 subnet_id\u003dsubnet_id).delete()"},{"line_number":184,"context_line":"        updated_types \u003d s.pop(\u0027service_type\u0027)"},{"line_number":185,"context_line":"        for service_type in updated_types:"},{"line_number":186,"context_line":"            new_type \u003d service_type_db.SubnetServiceType("}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_abaa5fd2","line":183,"in_reply_to":"bacf61ea_cddd3d72","updated":"2016-07-28 14:42:18.000000000","message":"Thanks for the explanation Kevin. I\u0027ll make the change and keep this in mind for the future.","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7f0b4e9fb5bd51d09c6dbbfdaf1267d5e72a33f2","unresolved":false,"context_lines":[{"line_number":181,"context_line":"    def _update_subnet_service_types(self, context, subnet_id, s):"},{"line_number":182,"context_line":"        old_types \u003d context.session.query("},{"line_number":183,"context_line":"                        service_type_db.SubnetServiceType).filter_by("},{"line_number":184,"context_line":"                            subnet_id\u003dsubnet_id)"},{"line_number":185,"context_line":"        for service_type in old_types:"},{"line_number":186,"context_line":"            context.session.delete(service_type)"},{"line_number":187,"context_line":"        updated_types \u003d s.pop(\u0027service_types\u0027)"}],"source_content_type":"text/x-python","patch_set":15,"id":"9ad45d7e_0007131d","line":184,"updated":"2016-08-09 00:14:52.000000000","message":"why don\u0027t you just issue a delete on old_types?\n\ne.g. \n\ncontext.session.query(service_type_db.SubnetServiceType).filter_by(\n                            subnet_id\u003dsubnet_id).delete()","commit_id":"26434fbca793be3b62e8e3d94cde62b470395f19"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"bea289e129a274865bb1ca3c441c23d6f24672f7","unresolved":false,"context_lines":[{"line_number":181,"context_line":"    def _update_subnet_service_types(self, context, subnet_id, s):"},{"line_number":182,"context_line":"        old_types \u003d context.session.query("},{"line_number":183,"context_line":"                        service_type_db.SubnetServiceType).filter_by("},{"line_number":184,"context_line":"                            subnet_id\u003dsubnet_id)"},{"line_number":185,"context_line":"        for service_type in old_types:"},{"line_number":186,"context_line":"            context.session.delete(service_type)"},{"line_number":187,"context_line":"        updated_types \u003d s.pop(\u0027service_types\u0027)"}],"source_content_type":"text/x-python","patch_set":15,"id":"9ad45d7e_583ba938","line":184,"in_reply_to":"9ad45d7e_0007131d","updated":"2016-08-09 12:32:04.000000000","message":"This is how I had it originally, but it was changed after a request from Kevin. Please see his comment on PS12.","commit_id":"26434fbca793be3b62e8e3d94cde62b470395f19"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"1d949093b9dc70b751e41b781061de1a1ac22dd1","unresolved":false,"context_lines":[{"line_number":181,"context_line":"    def _update_subnet_service_types(self, context, subnet_id, s):"},{"line_number":182,"context_line":"        old_types \u003d context.session.query("},{"line_number":183,"context_line":"                        service_type_db.SubnetServiceType).filter_by("},{"line_number":184,"context_line":"                            subnet_id\u003dsubnet_id)"},{"line_number":185,"context_line":"        for service_type in old_types:"},{"line_number":186,"context_line":"            context.session.delete(service_type)"},{"line_number":187,"context_line":"        updated_types \u003d s.pop(\u0027service_types\u0027)"}],"source_content_type":"text/x-python","patch_set":15,"id":"9ad45d7e_bb2c4d4b","line":184,"in_reply_to":"9ad45d7e_583ba938","updated":"2016-08-09 15:39:15.000000000","message":"Right, I figured that was the case, though I did not consider service types deletion something that required sql events to be processed. I\u0027ll double check with Kevin.\n\n/me noticed I did comment on this as well\n\n/me getting old.","commit_id":"26434fbca793be3b62e8e3d94cde62b470395f19"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"e6a909b5ab7f22098a8459bdc2d9b1b9c9ea68a5","unresolved":false,"context_lines":[{"line_number":181,"context_line":"    def _update_subnet_service_types(self, context, subnet_id, s):"},{"line_number":182,"context_line":"        old_types \u003d context.session.query("},{"line_number":183,"context_line":"                        service_type_db.SubnetServiceType).filter_by("},{"line_number":184,"context_line":"                            subnet_id\u003dsubnet_id)"},{"line_number":185,"context_line":"        for service_type in old_types:"},{"line_number":186,"context_line":"            context.session.delete(service_type)"},{"line_number":187,"context_line":"        updated_types \u003d s.pop(\u0027service_types\u0027)"}],"source_content_type":"text/x-python","patch_set":15,"id":"9ad45d7e_a330a302","line":184,"in_reply_to":"9ad45d7e_63cb4b54","updated":"2016-08-09 17:05:36.000000000","message":"I am on the fence here, but either way works.","commit_id":"26434fbca793be3b62e8e3d94cde62b470395f19"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"1ce272934a05b548e5bb0ddfa7812e1dbf80d917","unresolved":false,"context_lines":[{"line_number":181,"context_line":"    def _update_subnet_service_types(self, context, subnet_id, s):"},{"line_number":182,"context_line":"        old_types \u003d context.session.query("},{"line_number":183,"context_line":"                        service_type_db.SubnetServiceType).filter_by("},{"line_number":184,"context_line":"                            subnet_id\u003dsubnet_id)"},{"line_number":185,"context_line":"        for service_type in old_types:"},{"line_number":186,"context_line":"            context.session.delete(service_type)"},{"line_number":187,"context_line":"        updated_types \u003d s.pop(\u0027service_types\u0027)"}],"source_content_type":"text/x-python","patch_set":15,"id":"9ad45d7e_63cb4b54","line":184,"in_reply_to":"9ad45d7e_bb2c4d4b","updated":"2016-08-09 16:59:09.000000000","message":"@Kevin got *me* on this one too. I knew about it and still failed to notice. :)","commit_id":"26434fbca793be3b62e8e3d94cde62b470395f19"}],"neutron/db/migration/alembic_migrations/versions/newton/expand/a5648cfeeadf_add_subnet_service_types.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"11c103751d779d343d5f7965ec51063890da9606","unresolved":false,"context_lines":[{"line_number":15,"context_line":""},{"line_number":16,"context_line":"\"\"\"Add support for Subnet Service Types"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Revision ID: a5648cfeeadf"},{"line_number":19,"context_line":"Revises: c415aab1c048"},{"line_number":20,"context_line":"Create Date: 2016-03-15 18:00:00.190173"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"\"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"1aa78d24_da3286f1","line":19,"range":{"start_line":18,"start_character":0,"end_line":19,"end_character":21},"updated":"2016-07-06 20:35:49.000000000","message":"These\u0027ll probably get stale over the course of the review.  I usually just remove these three auto-generated comments because it is all redundant information.","commit_id":"8aebc1e7ead2c17270ce301c7f35a4f67aee3042"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"a423f4d23f24e85fafff187b5e548d7d933e9506","unresolved":false,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":"def upgrade():"},{"line_number":33,"context_line":"    op.create_table(\u0027subnet_service_types\u0027,"},{"line_number":34,"context_line":"        sa.Column(\u0027tenant_id\u0027, sa.String(length\u003d255), nullable\u003dTrue,"},{"line_number":35,"context_line":"                  index\u003dTrue),"},{"line_number":36,"context_line":"        sa.Column(\u0027id\u0027, sa.String(length\u003d36), nullable\u003dFalse),"},{"line_number":37,"context_line":"        sa.Column(\u0027subnet_id\u0027, sa.String(length\u003d36), nullable\u003dFalse),"}],"source_content_type":"text/x-python","patch_set":6,"id":"dada55a8_6b2b3eec","line":34,"updated":"2016-07-19 00:34:48.000000000","message":"Please be aware of [1].\n\n[1] https://review.openstack.org/#/c/335786/","commit_id":"2c36c20e4c818b660176efba810cbf438ba1e879"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"383ba0d5a90f23826816ad27399f0e434cde0d87","unresolved":false,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":"def upgrade():"},{"line_number":33,"context_line":"    op.create_table(\u0027subnet_service_types\u0027,"},{"line_number":34,"context_line":"        sa.Column(\u0027tenant_id\u0027, sa.String(length\u003d255), nullable\u003dTrue,"},{"line_number":35,"context_line":"                  index\u003dTrue),"},{"line_number":36,"context_line":"        sa.Column(\u0027id\u0027, sa.String(length\u003d36), nullable\u003dFalse),"},{"line_number":37,"context_line":"        sa.Column(\u0027subnet_id\u0027, sa.String(length\u003d36), nullable\u003dFalse),"}],"source_content_type":"text/x-python","patch_set":6,"id":"dada55a8_7fbac924","line":34,"in_reply_to":"dada55a8_6b2b3eec","updated":"2016-07-19 11:14:54.000000000","message":"I\u0027ll remove this entirely","commit_id":"2c36c20e4c818b660176efba810cbf438ba1e879"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"233ef0644b07089f058ef6db58a79970cfb20dbc","unresolved":false,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":"def upgrade():"},{"line_number":33,"context_line":"    op.create_table(\u0027subnet_service_types\u0027,"},{"line_number":34,"context_line":"        sa.Column(\u0027id\u0027, sa.String(length\u003d36), nullable\u003dFalse),"},{"line_number":35,"context_line":"        sa.Column(\u0027subnet_id\u0027, sa.String(length\u003d36), nullable\u003dFalse),"},{"line_number":36,"context_line":"        sa.Column(\u0027service_type\u0027, sa.String(length\u003d255), nullable\u003dFalse),"},{"line_number":37,"context_line":"        sa.ForeignKeyConstraint([\u0027subnet_id\u0027], [\u0027subnets.id\u0027],"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_616eb102","line":34,"updated":"2016-07-28 18:24:43.000000000","message":"why do we need an ID? subnet_id is already primary key and as far as I can tell we\u0027re not exposing this as top level resource [1, 2]. Was this discussed before?\n\n[1] https://review.openstack.org/#/c/337851/12/neutron/extensions/subnet_service_types.py\n[2] http://specs.openstack.org/openstack/neutron-specs/specs/newton/subnet-service-types.html#data-model-impact","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"62e03ae9f6bf5a636aeda5731ab00d8f334a1f4f","unresolved":false,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":"def upgrade():"},{"line_number":33,"context_line":"    op.create_table(\u0027subnet_service_types\u0027,"},{"line_number":34,"context_line":"        sa.Column(\u0027id\u0027, sa.String(length\u003d36), nullable\u003dFalse),"},{"line_number":35,"context_line":"        sa.Column(\u0027subnet_id\u0027, sa.String(length\u003d36), nullable\u003dFalse),"},{"line_number":36,"context_line":"        sa.Column(\u0027service_type\u0027, sa.String(length\u003d255), nullable\u003dFalse),"},{"line_number":37,"context_line":"        sa.ForeignKeyConstraint([\u0027subnet_id\u0027], [\u0027subnets.id\u0027],"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_de97578e","line":34,"in_reply_to":"bacf61ea_616eb102","updated":"2016-07-29 11:00:35.000000000","message":"Thanks for pointing this out. I somehow missed this along the way.","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"671f32363a902788f3ce09b53f927d12e85a3cc9","unresolved":false,"context_lines":[{"line_number":32,"context_line":"def upgrade():"},{"line_number":33,"context_line":"    op.create_table(\u0027subnet_service_types\u0027,"},{"line_number":34,"context_line":"        sa.Column(\u0027id\u0027, sa.String(length\u003d36), nullable\u003dFalse),"},{"line_number":35,"context_line":"        sa.Column(\u0027subnet_id\u0027, sa.String(length\u003d36), nullable\u003dFalse),"},{"line_number":36,"context_line":"        sa.Column(\u0027service_type\u0027, sa.String(length\u003d255), nullable\u003dFalse),"},{"line_number":37,"context_line":"        sa.ForeignKeyConstraint([\u0027subnet_id\u0027], [\u0027subnets.id\u0027],"},{"line_number":38,"context_line":"                                ondelete\u003d\u0027CASCADE\u0027),"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_ede4813c","line":35,"updated":"2016-07-28 04:16:47.000000000","message":"i think it would be a good idea to create an index on subnet_id since it will almost always be joined using that column","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"c8d5980e5035d970ae9478b510f99d81e5e098d6","unresolved":false,"context_lines":[{"line_number":32,"context_line":"def upgrade():"},{"line_number":33,"context_line":"    op.create_table(\u0027subnet_service_types\u0027,"},{"line_number":34,"context_line":"        sa.Column(\u0027id\u0027, sa.String(length\u003d36), nullable\u003dFalse),"},{"line_number":35,"context_line":"        sa.Column(\u0027subnet_id\u0027, sa.String(length\u003d36), nullable\u003dFalse),"},{"line_number":36,"context_line":"        sa.Column(\u0027service_type\u0027, sa.String(length\u003d255), nullable\u003dFalse),"},{"line_number":37,"context_line":"        sa.ForeignKeyConstraint([\u0027subnet_id\u0027], [\u0027subnets.id\u0027],"},{"line_number":38,"context_line":"                                ondelete\u003d\u0027CASCADE\u0027),"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_2cb5c6ed","line":35,"in_reply_to":"bacf61ea_0c456216","updated":"2016-07-28 22:19:35.000000000","message":"I thought I should clarify a little just to be sure.\n\nIf you make (subnet_id, service_type) the primary key, then you\u0027d get the index for free (as long as subnet_id is first) because primary keys are implemented with an index to aid in enforcing uniqueness.\n\nSo, I wasn\u0027t really suggesting setting this primary key in addition to setting the index. I meant to suggest using the primary key instead of a simple index.","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"62e03ae9f6bf5a636aeda5731ab00d8f334a1f4f","unresolved":false,"context_lines":[{"line_number":32,"context_line":"def upgrade():"},{"line_number":33,"context_line":"    op.create_table(\u0027subnet_service_types\u0027,"},{"line_number":34,"context_line":"        sa.Column(\u0027id\u0027, sa.String(length\u003d36), nullable\u003dFalse),"},{"line_number":35,"context_line":"        sa.Column(\u0027subnet_id\u0027, sa.String(length\u003d36), nullable\u003dFalse),"},{"line_number":36,"context_line":"        sa.Column(\u0027service_type\u0027, sa.String(length\u003d255), nullable\u003dFalse),"},{"line_number":37,"context_line":"        sa.ForeignKeyConstraint([\u0027subnet_id\u0027], [\u0027subnets.id\u0027],"},{"line_number":38,"context_line":"                                ondelete\u003d\u0027CASCADE\u0027),"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_feedfbef","line":35,"in_reply_to":"bacf61ea_2cb5c6ed","updated":"2016-07-29 11:00:35.000000000","message":"I had gone ahead and added the index Kevin suggested, but if we don\u0027t need it anymore with the updated primary key then I\u0027ll remove it. Hopefully Kevin will be happy with that. Thanks.","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"a2532181a87d06a974eb7fc6fc5402e13f997313","unresolved":false,"context_lines":[{"line_number":32,"context_line":"def upgrade():"},{"line_number":33,"context_line":"    op.create_table(\u0027subnet_service_types\u0027,"},{"line_number":34,"context_line":"        sa.Column(\u0027id\u0027, sa.String(length\u003d36), nullable\u003dFalse),"},{"line_number":35,"context_line":"        sa.Column(\u0027subnet_id\u0027, sa.String(length\u003d36), nullable\u003dFalse),"},{"line_number":36,"context_line":"        sa.Column(\u0027service_type\u0027, sa.String(length\u003d255), nullable\u003dFalse),"},{"line_number":37,"context_line":"        sa.ForeignKeyConstraint([\u0027subnet_id\u0027], [\u0027subnets.id\u0027],"},{"line_number":38,"context_line":"                                ondelete\u003d\u0027CASCADE\u0027),"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_0c456216","line":35,"in_reply_to":"bacf61ea_cb8353cb","updated":"2016-07-28 22:13:10.000000000","message":"That\u0027s good feedback. You might also consider making the primary key (subnet_id, service_type) to get the unique constraint out of it to. Especially given Armando\u0027s feedback that you don\u0027t need id.","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"dce97d6e567d77f9fabf2cae3ce45fd4aed08ca1","unresolved":false,"context_lines":[{"line_number":32,"context_line":"def upgrade():"},{"line_number":33,"context_line":"    op.create_table(\u0027subnet_service_types\u0027,"},{"line_number":34,"context_line":"        sa.Column(\u0027id\u0027, sa.String(length\u003d36), nullable\u003dFalse),"},{"line_number":35,"context_line":"        sa.Column(\u0027subnet_id\u0027, sa.String(length\u003d36), nullable\u003dFalse),"},{"line_number":36,"context_line":"        sa.Column(\u0027service_type\u0027, sa.String(length\u003d255), nullable\u003dFalse),"},{"line_number":37,"context_line":"        sa.ForeignKeyConstraint([\u0027subnet_id\u0027], [\u0027subnets.id\u0027],"},{"line_number":38,"context_line":"                                ondelete\u003d\u0027CASCADE\u0027),"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_cb8353cb","line":35,"in_reply_to":"bacf61ea_ede4813c","updated":"2016-07-28 14:42:18.000000000","message":"Done","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"d1375e4e4284cbeedbe9d18f5b0a83b9f1d56fd9","unresolved":false,"context_lines":[{"line_number":38,"context_line":"        sa.PrimaryKeyConstraint(\u0027subnet_id\u0027, \u0027service_type\u0027)"},{"line_number":39,"context_line":"    )"},{"line_number":40,"context_line":"    op.create_index(\u0027ix_subnet_service_types_subnet_id\u0027,"},{"line_number":41,"context_line":"                    \u0027subnet_service_types\u0027, [\u0027subnet_id\u0027], unique\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":13,"id":"bacf61ea_98120556","line":41,"range":{"start_line":41,"start_character":46,"end_line":41,"end_character":55},"updated":"2016-07-29 15:31:46.000000000","message":"This is redundant with the primary key. It should be removed.","commit_id":"ab9bf13796f4f129ccfde36a0a4f455ce738ad5b"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"e4688c743c5ce29b78f15b66492fb6616d1ab1f3","unresolved":false,"context_lines":[{"line_number":38,"context_line":"        sa.PrimaryKeyConstraint(\u0027subnet_id\u0027, \u0027service_type\u0027)"},{"line_number":39,"context_line":"    )"},{"line_number":40,"context_line":"    op.create_index(\u0027ix_subnet_service_types_subnet_id\u0027,"},{"line_number":41,"context_line":"                    \u0027subnet_service_types\u0027, [\u0027subnet_id\u0027], unique\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":13,"id":"bacf61ea_dbb47736","line":41,"range":{"start_line":41,"start_character":46,"end_line":41,"end_character":55},"in_reply_to":"bacf61ea_98120556","updated":"2016-07-29 15:35:00.000000000","message":"Damn! Thought I\u0027d gotten rid of it everywhere. New patch incoming","commit_id":"ab9bf13796f4f129ccfde36a0a4f455ce738ad5b"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7f0b4e9fb5bd51d09c6dbbfdaf1267d5e72a33f2","unresolved":false,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":"def upgrade():"},{"line_number":33,"context_line":"    op.create_table(\u0027subnet_service_types\u0027,"},{"line_number":34,"context_line":"        sa.Column(\u0027subnet_id\u0027, sa.String(length\u003d36), nullable\u003dFalse),"},{"line_number":35,"context_line":"        sa.Column(\u0027service_type\u0027, sa.String(length\u003d255), nullable\u003dFalse),"},{"line_number":36,"context_line":"        sa.ForeignKeyConstraint([\u0027subnet_id\u0027], [\u0027subnets.id\u0027],"},{"line_number":37,"context_line":"                                ondelete\u003d\u0027CASCADE\u0027),"}],"source_content_type":"text/x-python","patch_set":15,"id":"9ad45d7e_9b109250","line":34,"updated":"2016-08-09 00:14:52.000000000","message":"nullable\u003dFalse can be omitted for both.","commit_id":"26434fbca793be3b62e8e3d94cde62b470395f19"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"bea289e129a274865bb1ca3c441c23d6f24672f7","unresolved":false,"context_lines":[{"line_number":31,"context_line":""},{"line_number":32,"context_line":"def upgrade():"},{"line_number":33,"context_line":"    op.create_table(\u0027subnet_service_types\u0027,"},{"line_number":34,"context_line":"        sa.Column(\u0027subnet_id\u0027, sa.String(length\u003d36), nullable\u003dFalse),"},{"line_number":35,"context_line":"        sa.Column(\u0027service_type\u0027, sa.String(length\u003d255), nullable\u003dFalse),"},{"line_number":36,"context_line":"        sa.ForeignKeyConstraint([\u0027subnet_id\u0027], [\u0027subnets.id\u0027],"},{"line_number":37,"context_line":"                                ondelete\u003d\u0027CASCADE\u0027),"}],"source_content_type":"text/x-python","patch_set":15,"id":"9ad45d7e_984a41ab","line":34,"in_reply_to":"9ad45d7e_9b109250","updated":"2016-08-09 12:32:04.000000000","message":"Done","commit_id":"26434fbca793be3b62e8e3d94cde62b470395f19"}],"neutron/db/models_v2.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"11c103751d779d343d5f7965ec51063890da9606","unresolved":false,"context_lines":[{"line_number":228,"context_line":"        foreign_keys\u003d\u0027Subnet.network_id\u0027,"},{"line_number":229,"context_line":"        primaryjoin\u003d\u0027Subnet.network_id\u003d\u003dNetworkRBAC.object_id\u0027)"},{"line_number":230,"context_line":"    # subnet service type entries live in their own table"},{"line_number":231,"context_line":"    service_type \u003d orm.relationship("},{"line_number":232,"context_line":"        subnet_service_type_db_models.SubnetServiceType,"},{"line_number":233,"context_line":"        lazy\u003d\u0027joined\u0027,"},{"line_number":234,"context_line":"        uselist\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":1,"id":"1aa78d24_0e7690a3","line":231,"range":{"start_line":231,"start_character":4,"end_line":231,"end_character":35},"updated":"2016-07-06 20:35:49.000000000","message":"I think you can stick this relationship in the other table to avoid piling on more to the subnet model.  I think you just need to add a backref.  I just know that if Armando sees this, he\u0027ll not like the coupling this introduces in the code. :)","commit_id":"8aebc1e7ead2c17270ce301c7f35a4f67aee3042"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"af91fe95276994688f4104c0ebf39663bc96f870","unresolved":false,"context_lines":[{"line_number":228,"context_line":"        foreign_keys\u003d\u0027Subnet.network_id\u0027,"},{"line_number":229,"context_line":"        primaryjoin\u003d\u0027Subnet.network_id\u003d\u003dNetworkRBAC.object_id\u0027)"},{"line_number":230,"context_line":"    # subnet service type entries live in their own table"},{"line_number":231,"context_line":"    service_type \u003d orm.relationship("},{"line_number":232,"context_line":"        subnet_service_type_db_models.SubnetServiceType,"},{"line_number":233,"context_line":"        lazy\u003d\u0027joined\u0027,"},{"line_number":234,"context_line":"        uselist\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":1,"id":"1aa78d24_a8a666be","line":231,"range":{"start_line":231,"start_character":4,"end_line":231,"end_character":35},"in_reply_to":"1aa78d24_0e7690a3","updated":"2016-07-12 10:37:28.000000000","message":"I\u0027ve done some digging but I can\u0027t see a way to do this without adding to the subnet model. [1] doesn\u0027t make it look like a backref would help here. Can you elaborate on your idea? Thanks.\n\n[1] http://docs.sqlalchemy.org/en/latest/orm/backref.html#linking-relationships-with-backref","commit_id":"8aebc1e7ead2c17270ce301c7f35a4f67aee3042"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"afec8e5058b6d207bd94703080c448b567540e73","unresolved":false,"context_lines":[{"line_number":228,"context_line":"        foreign_keys\u003d\u0027Subnet.network_id\u0027,"},{"line_number":229,"context_line":"        primaryjoin\u003d\u0027Subnet.network_id\u003d\u003dNetworkRBAC.object_id\u0027)"},{"line_number":230,"context_line":"    # subnet service type entries live in their own table"},{"line_number":231,"context_line":"    service_type \u003d orm.relationship("},{"line_number":232,"context_line":"        subnet_service_type_db_models.SubnetServiceType,"},{"line_number":233,"context_line":"        lazy\u003d\u0027joined\u0027,"},{"line_number":234,"context_line":"        uselist\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":1,"id":"1aa78d24_a380fc8b","line":231,"range":{"start_line":231,"start_character":4,"end_line":231,"end_character":35},"in_reply_to":"1aa78d24_0e7690a3","updated":"2016-07-07 18:08:07.000000000","message":"Yes, that would make more sense.","commit_id":"8aebc1e7ead2c17270ce301c7f35a4f67aee3042"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"ce91caf5cdb1eff641bfcfca9d249b1acf2c39cc","unresolved":false,"context_lines":[{"line_number":228,"context_line":"        foreign_keys\u003d\u0027Subnet.network_id\u0027,"},{"line_number":229,"context_line":"        primaryjoin\u003d\u0027Subnet.network_id\u003d\u003dNetworkRBAC.object_id\u0027)"},{"line_number":230,"context_line":"    # subnet service type entries live in their own table"},{"line_number":231,"context_line":"    service_type \u003d orm.relationship("},{"line_number":232,"context_line":"        subnet_service_type_db_models.SubnetServiceType,"},{"line_number":233,"context_line":"        backref\u003d\u0027subnet\u0027,"},{"line_number":234,"context_line":"        lazy\u003d\u0027joined\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_90f3ad72","line":231,"updated":"2016-07-12 21:56:18.000000000","message":"I still think this can be moved to the other file.  See my comment there.  I could be wrong, I haven\u0027t tried it.  But, I\u0027m pretty sure I\u0027ve seen this before.  I think trunks [1][2] is an example I\u0027ve seen recently.\n\n [1] https://github.com/openstack/neutron/blob/ed58510023d7bbeb2daf61bb1cbdb4e5e4537c0a/neutron/services/trunk/models.py#L32\n [2] https://review.openstack.org/#/c/334700/11/neutron/services/trunk/plugin.py@37","commit_id":"4038cb197e938a11ae7f44795746540e10108f2c"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"491f718790277768d1dd5bb47806d5d1f2f2e7ff","unresolved":false,"context_lines":[{"line_number":228,"context_line":"        foreign_keys\u003d\u0027Subnet.network_id\u0027,"},{"line_number":229,"context_line":"        primaryjoin\u003d\u0027Subnet.network_id\u003d\u003dNetworkRBAC.object_id\u0027)"},{"line_number":230,"context_line":"    # subnet service type entries live in their own table"},{"line_number":231,"context_line":"    service_type \u003d orm.relationship("},{"line_number":232,"context_line":"        subnet_service_type_db_models.SubnetServiceType,"},{"line_number":233,"context_line":"        backref\u003d\u0027subnet\u0027,"},{"line_number":234,"context_line":"        lazy\u003d\u0027joined\u0027,"}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_d2deaf0f","line":231,"in_reply_to":"1aa78d24_90f3ad72","updated":"2016-07-13 16:17:28.000000000","message":"Thanks for the pointer to the Trunks model, that looks very similar to what we\u0027re trying to do.","commit_id":"4038cb197e938a11ae7f44795746540e10108f2c"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"ce91caf5cdb1eff641bfcfca9d249b1acf2c39cc","unresolved":false,"context_lines":[{"line_number":233,"context_line":"        backref\u003d\u0027subnet\u0027,"},{"line_number":234,"context_line":"        lazy\u003d\u0027joined\u0027,"},{"line_number":235,"context_line":"        uselist\u003dTrue,"},{"line_number":236,"context_line":"        foreign_keys\u003d\u0027Subnet.id\u0027,"},{"line_number":237,"context_line":"        primaryjoin\u003d\u0027Subnet.id\u003d\u003dSubnetServiceType.subnet_id\u0027)"},{"line_number":238,"context_line":""},{"line_number":239,"context_line":""},{"line_number":240,"context_line":"class SubnetPoolPrefix(model_base.BASEV2):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_50df85d5","line":237,"range":{"start_line":236,"start_character":8,"end_line":237,"end_character":60},"updated":"2016-07-12 21:56:18.000000000","message":"These should be computed automatically by sqlalchemy [1] based on the FK declaration in the other table.\n\n [1] http://docs.sqlalchemy.org/en/latest/orm/relationship_api.html#sqlalchemy.orm.relationship.params.primaryjoin","commit_id":"4038cb197e938a11ae7f44795746540e10108f2c"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"491f718790277768d1dd5bb47806d5d1f2f2e7ff","unresolved":false,"context_lines":[{"line_number":233,"context_line":"        backref\u003d\u0027subnet\u0027,"},{"line_number":234,"context_line":"        lazy\u003d\u0027joined\u0027,"},{"line_number":235,"context_line":"        uselist\u003dTrue,"},{"line_number":236,"context_line":"        foreign_keys\u003d\u0027Subnet.id\u0027,"},{"line_number":237,"context_line":"        primaryjoin\u003d\u0027Subnet.id\u003d\u003dSubnetServiceType.subnet_id\u0027)"},{"line_number":238,"context_line":""},{"line_number":239,"context_line":""},{"line_number":240,"context_line":"class SubnetPoolPrefix(model_base.BASEV2):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_92742736","line":237,"range":{"start_line":236,"start_character":8,"end_line":237,"end_character":60},"in_reply_to":"1aa78d24_50df85d5","updated":"2016-07-13 16:17:28.000000000","message":"Done","commit_id":"4038cb197e938a11ae7f44795746540e10108f2c"}],"neutron/db/subnet_service_type_db_models.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"11c103751d779d343d5f7965ec51063890da9606","unresolved":false,"context_lines":[{"line_number":18,"context_line":"from neutron.db import model_base"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"class SubnetServiceTypeColumns(model_base.HasId, model_base.HasTenant):"},{"line_number":22,"context_line":"    subnet_id \u003d sa.Column(sa.String(length\u003d36), nullable\u003dFalse)"},{"line_number":23,"context_line":"    service_type \u003d sa.Column(sa.String(length\u003d36), nullable\u003dFalse)"},{"line_number":24,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1aa78d24_4ee538da","line":21,"range":{"start_line":21,"start_character":6,"end_line":21,"end_character":30},"updated":"2016-07-06 20:35:49.000000000","message":"What\u0027s the extra class is for?","commit_id":"8aebc1e7ead2c17270ce301c7f35a4f67aee3042"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"afec8e5058b6d207bd94703080c448b567540e73","unresolved":false,"context_lines":[{"line_number":18,"context_line":"from neutron.db import model_base"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"class SubnetServiceTypeColumns(model_base.HasId, model_base.HasTenant):"},{"line_number":22,"context_line":"    subnet_id \u003d sa.Column(sa.String(length\u003d36), nullable\u003dFalse)"},{"line_number":23,"context_line":"    service_type \u003d sa.Column(sa.String(length\u003d36), nullable\u003dFalse)"},{"line_number":24,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1aa78d24_430af826","line":21,"range":{"start_line":21,"start_character":6,"end_line":21,"end_character":30},"in_reply_to":"1aa78d24_4ee538da","updated":"2016-07-07 18:08:07.000000000","message":"I had started modeling this based on the RBAC code, but wasn\u0027t done.","commit_id":"8aebc1e7ead2c17270ce301c7f35a4f67aee3042"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"11c103751d779d343d5f7965ec51063890da9606","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"class SubnetServiceTypeColumns(model_base.HasId, model_base.HasTenant):"},{"line_number":22,"context_line":"    subnet_id \u003d sa.Column(sa.String(length\u003d36), nullable\u003dFalse)"},{"line_number":23,"context_line":"    service_type \u003d sa.Column(sa.String(length\u003d36), nullable\u003dFalse)"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"class SubnetServiceType(SubnetServiceTypeColumns, model_base.BASEV2):"}],"source_content_type":"text/x-python","patch_set":1,"id":"1aa78d24_0ec97026","line":23,"range":{"start_line":23,"start_character":46,"end_line":23,"end_character":48},"updated":"2016-07-06 20:35:49.000000000","message":"We should use at least attr.DEVICE_OWNER_MAX_LEN [1].  36 is not going to be enough unless you\u0027ve got another indirection in the works.\n\n [1] https://github.com/openstack/neutron/blob/4a98b31b6ae5d59a64906224d3a8452d956667a6/neutron/api/v2/attributes.py#L41","commit_id":"8aebc1e7ead2c17270ce301c7f35a4f67aee3042"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"af91fe95276994688f4104c0ebf39663bc96f870","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"class SubnetServiceTypeColumns(model_base.HasId, model_base.HasTenant):"},{"line_number":22,"context_line":"    subnet_id \u003d sa.Column(sa.String(length\u003d36), nullable\u003dFalse)"},{"line_number":23,"context_line":"    service_type \u003d sa.Column(sa.String(length\u003d36), nullable\u003dFalse)"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"class SubnetServiceType(SubnetServiceTypeColumns, model_base.BASEV2):"}],"source_content_type":"text/x-python","patch_set":1,"id":"1aa78d24_889c4a6b","line":23,"range":{"start_line":23,"start_character":46,"end_line":23,"end_character":48},"in_reply_to":"1aa78d24_0ec97026","updated":"2016-07-12 10:37:28.000000000","message":"Done","commit_id":"8aebc1e7ead2c17270ce301c7f35a4f67aee3042"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"afec8e5058b6d207bd94703080c448b567540e73","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"class SubnetServiceTypeColumns(model_base.HasId, model_base.HasTenant):"},{"line_number":22,"context_line":"    subnet_id \u003d sa.Column(sa.String(length\u003d36), nullable\u003dFalse)"},{"line_number":23,"context_line":"    service_type \u003d sa.Column(sa.String(length\u003d36), nullable\u003dFalse)"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"class SubnetServiceType(SubnetServiceTypeColumns, model_base.BASEV2):"}],"source_content_type":"text/x-python","patch_set":1,"id":"1aa78d24_630f3437","line":23,"range":{"start_line":23,"start_character":46,"end_line":23,"end_character":48},"in_reply_to":"1aa78d24_0ec97026","updated":"2016-07-07 18:08:07.000000000","message":"Yes, will remember to check when John sends an update.","commit_id":"8aebc1e7ead2c17270ce301c7f35a4f67aee3042"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"ce91caf5cdb1eff641bfcfca9d249b1acf2c39cc","unresolved":false,"context_lines":[{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    __tablename__ \u003d \"subnet_service_types\""},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    subnet_id \u003d _subnet_id_column(\u0027subnets.id\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_70c3a138","line":39,"updated":"2016-07-12 21:56:18.000000000","message":"I thought we could stick an orm relationship here that allowed referencing the service_type from the subnet *without* adding to the subnet model.\n\n  subnet \u003d orm.relationship(\n      models_v2.Subnet,\n      backref\u003dorm.backref(\u0027service_type\u0027, lazy\u003d\u0027joined\u0027, uselist\u003dTrue))","commit_id":"4038cb197e938a11ae7f44795746540e10108f2c"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"491f718790277768d1dd5bb47806d5d1f2f2e7ff","unresolved":false,"context_lines":[{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    __tablename__ \u003d \"subnet_service_types\""},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    subnet_id \u003d _subnet_id_column(\u0027subnets.id\u0027)"}],"source_content_type":"text/x-python","patch_set":3,"id":"1aa78d24_f2e76be0","line":39,"in_reply_to":"1aa78d24_70c3a138","updated":"2016-07-13 16:17:28.000000000","message":"Thanks, I\u0027ll give this a go.","commit_id":"4038cb197e938a11ae7f44795746540e10108f2c"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"6dcfa3f4c5db40465e159d99c3b57bdfa21be120","unresolved":false,"context_lines":[{"line_number":21,"context_line":"from neutron.db import models_v2"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"class SubnetServiceTypeColumns(model_base.HasId, model_base.HasTenant):"},{"line_number":25,"context_line":"    service_type \u003d sa.Column(sa.String("},{"line_number":26,"context_line":"                                length\u003dattributes.DEVICE_OWNER_MAX_LEN),"},{"line_number":27,"context_line":"                             nullable\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":5,"id":"1aa78d24_6a6184a8","line":24,"range":{"start_line":24,"start_character":6,"end_line":24,"end_character":30},"updated":"2016-07-14 16:47:01.000000000","message":"I have to say, I still don\u0027t understand the need to split this table in to two classes.  Is there a reason?  If not, I think I\u0027d prefer to just collapse the classes in to one.","commit_id":"dd2520b84d037d97d005319c3c2f16d6dd9a94b1"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"61158d387282c13a9182365736d57743e53c3c42","unresolved":false,"context_lines":[{"line_number":21,"context_line":"from neutron.db import models_v2"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"class SubnetServiceTypeColumns(model_base.HasId, model_base.HasTenant):"},{"line_number":25,"context_line":"    service_type \u003d sa.Column(sa.String("},{"line_number":26,"context_line":"                                length\u003dattributes.DEVICE_OWNER_MAX_LEN),"},{"line_number":27,"context_line":"                             nullable\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":5,"id":"fad55976_901c1c79","line":24,"range":{"start_line":24,"start_character":6,"end_line":24,"end_character":30},"in_reply_to":"1aa78d24_6a6184a8","updated":"2016-07-15 15:50:02.000000000","message":"No reason other than style. I\u0027ll fold it all into the one class.","commit_id":"dd2520b84d037d97d005319c3c2f16d6dd9a94b1"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"a423f4d23f24e85fafff187b5e548d7d933e9506","unresolved":false,"context_lines":[{"line_number":23,"context_line":""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"class SubnetServiceType(model_base.BASEV2, model_base.HasId,"},{"line_number":26,"context_line":"                        model_base.HasTenant):"},{"line_number":27,"context_line":"    \"\"\"Subnet Service Types table\"\"\""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    __tablename__ \u003d \"subnet_service_types\""}],"source_content_type":"text/x-python","patch_set":6,"id":"dada55a8_8b1152af","line":26,"updated":"2016-07-19 00:34:48.000000000","message":"Not sure if this was brought up in the past but do you need Tenant here if we\u0027re simply using this model to store an attribute on the subnet?","commit_id":"2c36c20e4c818b660176efba810cbf438ba1e879"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"383ba0d5a90f23826816ad27399f0e434cde0d87","unresolved":false,"context_lines":[{"line_number":23,"context_line":""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"class SubnetServiceType(model_base.BASEV2, model_base.HasId,"},{"line_number":26,"context_line":"                        model_base.HasTenant):"},{"line_number":27,"context_line":"    \"\"\"Subnet Service Types table\"\"\""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"    __tablename__ \u003d \"subnet_service_types\""}],"source_content_type":"text/x-python","patch_set":6,"id":"dada55a8_3f169255","line":26,"in_reply_to":"dada55a8_8b1152af","updated":"2016-07-19 11:14:54.000000000","message":"Actually, probably not. Even though we\u0027ll be querying this table directly, we should only ever need to filter it by subnet id rather than project id. Unless anyone else can think of a scenario where we\u0027d need project id I\u0027ll go ahead and remove it.","commit_id":"2c36c20e4c818b660176efba810cbf438ba1e879"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"0443d77a35b18a421dd0a25b5d18fe28062f8a80","unresolved":false,"context_lines":[{"line_number":31,"context_line":"                          sa.ForeignKey(\u0027subnets.id\u0027, ondelete\u003d\"CASCADE\"),"},{"line_number":32,"context_line":"                          nullable\u003dFalse)"},{"line_number":33,"context_line":"    service_type \u003d sa.Column(sa.String("},{"line_number":34,"context_line":"                                length\u003dattributes.DEVICE_OWNER_MAX_LEN),"},{"line_number":35,"context_line":"                             nullable\u003dFalse)"},{"line_number":36,"context_line":"    subnet \u003d orm.relationship(models_v2.Subnet,"},{"line_number":37,"context_line":"                              backref\u003dorm.backref(\u0027service_type\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"dada55a8_132106b5","line":34,"updated":"2016-07-19 11:33:25.000000000","message":"why not just 255\nthe device owner here is misleading","commit_id":"b733ccd6b5619f69668563a39204eb66a2ab02a3"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"11664c1f7e82659b67d96ec77b4b1c374536a436","unresolved":false,"context_lines":[{"line_number":31,"context_line":"                          sa.ForeignKey(\u0027subnets.id\u0027, ondelete\u003d\"CASCADE\"),"},{"line_number":32,"context_line":"                          nullable\u003dFalse)"},{"line_number":33,"context_line":"    service_type \u003d sa.Column(sa.String("},{"line_number":34,"context_line":"                                length\u003dattributes.DEVICE_OWNER_MAX_LEN),"},{"line_number":35,"context_line":"                             nullable\u003dFalse)"},{"line_number":36,"context_line":"    subnet \u003d orm.relationship(models_v2.Subnet,"},{"line_number":37,"context_line":"                              backref\u003dorm.backref(\u0027service_type\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"dada55a8_8e8b091e","line":34,"in_reply_to":"dada55a8_132106b5","updated":"2016-07-19 11:49:00.000000000","message":"Why just 255? The reason it\u0027s 255 is because we\u0027re storing device owners here, so I think it makes sense to use the attribute. What about it do you find misleading?","commit_id":"b733ccd6b5619f69668563a39204eb66a2ab02a3"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"9454edf998abcd34a9bd5890b9af11c3acdecf61","unresolved":false,"context_lines":[{"line_number":31,"context_line":"                          sa.ForeignKey(\u0027subnets.id\u0027, ondelete\u003d\"CASCADE\"),"},{"line_number":32,"context_line":"                          nullable\u003dFalse)"},{"line_number":33,"context_line":"    service_type \u003d sa.Column(sa.String("},{"line_number":34,"context_line":"                                length\u003dattributes.DEVICE_OWNER_MAX_LEN),"},{"line_number":35,"context_line":"                             nullable\u003dFalse)"},{"line_number":36,"context_line":"    subnet \u003d orm.relationship(models_v2.Subnet,"},{"line_number":37,"context_line":"                              backref\u003dorm.backref(\u0027service_type\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"dada55a8_27e1289b","line":34,"in_reply_to":"dada55a8_33735c4f","updated":"2016-07-20 11:43:19.000000000","message":"Sure, I\u0027ll add a note","commit_id":"b733ccd6b5619f69668563a39204eb66a2ab02a3"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"e86e07d5d9880d3fed1a60e618a054c0dad7e45b","unresolved":false,"context_lines":[{"line_number":31,"context_line":"                          sa.ForeignKey(\u0027subnets.id\u0027, ondelete\u003d\"CASCADE\"),"},{"line_number":32,"context_line":"                          nullable\u003dFalse)"},{"line_number":33,"context_line":"    service_type \u003d sa.Column(sa.String("},{"line_number":34,"context_line":"                                length\u003dattributes.DEVICE_OWNER_MAX_LEN),"},{"line_number":35,"context_line":"                             nullable\u003dFalse)"},{"line_number":36,"context_line":"    subnet \u003d orm.relationship(models_v2.Subnet,"},{"line_number":37,"context_line":"                              backref\u003dorm.backref(\u0027service_type\u0027,"}],"source_content_type":"text/x-python","patch_set":7,"id":"dada55a8_33735c4f","line":34,"in_reply_to":"dada55a8_8e8b091e","updated":"2016-07-19 23:05:49.000000000","message":"It might warrant a note saying why this attribute is being used for this field just so that the passer-by doesn\u0027t have to scratch his/her head.  But, I agree this is the correct constant to use.","commit_id":"b733ccd6b5619f69668563a39204eb66a2ab02a3"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"671f32363a902788f3ce09b53f927d12e85a3cc9","unresolved":false,"context_lines":[{"line_number":35,"context_line":"                                length\u003dattributes.DEVICE_OWNER_MAX_LEN),"},{"line_number":36,"context_line":"                             nullable\u003dFalse)"},{"line_number":37,"context_line":"    subnet \u003d orm.relationship(models_v2.Subnet,"},{"line_number":38,"context_line":"                              backref\u003dorm.backref(\u0027service_type\u0027,"},{"line_number":39,"context_line":"                                                  lazy\u003d\u0027joined\u0027,"},{"line_number":40,"context_line":"                                                  uselist\u003dTrue))"},{"line_number":41,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_adda8976","line":38,"updated":"2016-07-28 04:16:47.000000000","message":"you probably want to call the backref \u0027service_types\u0027 since it\u0027s a 1 to many relationship","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"dea2e34b289f6f01ae7a05505e35053364a6352f","unresolved":false,"context_lines":[{"line_number":35,"context_line":"                                length\u003dattributes.DEVICE_OWNER_MAX_LEN),"},{"line_number":36,"context_line":"                             nullable\u003dFalse)"},{"line_number":37,"context_line":"    subnet \u003d orm.relationship(models_v2.Subnet,"},{"line_number":38,"context_line":"                              backref\u003dorm.backref(\u0027service_type\u0027,"},{"line_number":39,"context_line":"                                                  lazy\u003d\u0027joined\u0027,"},{"line_number":40,"context_line":"                                                  uselist\u003dTrue))"},{"line_number":41,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_4c9f5a50","line":38,"in_reply_to":"bacf61ea_36fd2f3b","updated":"2016-07-28 22:24:09.000000000","message":"No objections, this is good feedback. I sort of became numb to the plurality thing because we discussed it both ways in the spec before decided on many to 1.","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"dce97d6e567d77f9fabf2cae3ce45fd4aed08ca1","unresolved":false,"context_lines":[{"line_number":35,"context_line":"                                length\u003dattributes.DEVICE_OWNER_MAX_LEN),"},{"line_number":36,"context_line":"                             nullable\u003dFalse)"},{"line_number":37,"context_line":"    subnet \u003d orm.relationship(models_v2.Subnet,"},{"line_number":38,"context_line":"                              backref\u003dorm.backref(\u0027service_type\u0027,"},{"line_number":39,"context_line":"                                                  lazy\u003d\u0027joined\u0027,"},{"line_number":40,"context_line":"                                                  uselist\u003dTrue))"},{"line_number":41,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_36fd2f3b","line":38,"in_reply_to":"bacf61ea_adda8976","updated":"2016-07-28 14:42:18.000000000","message":"Yeah, I\u0027ve been thinking about this for a while actually. The spec uses \u0027service_type\u0027 but that hasn\u0027t felt right in the implementation since it\u0027s representing a list. I\u0027ll update it, hopefully there won\u0027t be any objections.","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"233ef0644b07089f058ef6db58a79970cfb20dbc","unresolved":false,"context_lines":[{"line_number":44,"context_line":"    \"\"\"Mixin class to extend subnet with service type attribute\"\"\""},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    def _extend_subnet_service_type(self, subnet_res, subnet_db):"},{"line_number":47,"context_line":"        subnet_res[\u0027service_type\u0027] \u003d [service_type[\u0027service_type\u0027] for"},{"line_number":48,"context_line":"                                      service_type in"},{"line_number":49,"context_line":"                                      subnet_db[\u0027service_type\u0027]]"},{"line_number":50,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_e107411a","line":47,"updated":"2016-07-28 18:24:43.000000000","message":"if this is a meant to be a list of types shouldn\u0027t the extension attribute be service_typeS? This seems overlooked on the spec proposal.","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"62e03ae9f6bf5a636aeda5731ab00d8f334a1f4f","unresolved":false,"context_lines":[{"line_number":44,"context_line":"    \"\"\"Mixin class to extend subnet with service type attribute\"\"\""},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    def _extend_subnet_service_type(self, subnet_res, subnet_db):"},{"line_number":47,"context_line":"        subnet_res[\u0027service_type\u0027] \u003d [service_type[\u0027service_type\u0027] for"},{"line_number":48,"context_line":"                                      service_type in"},{"line_number":49,"context_line":"                                      subnet_db[\u0027service_type\u0027]]"},{"line_number":50,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_b8a6fa0d","line":47,"in_reply_to":"bacf61ea_e107411a","updated":"2016-07-29 11:00:35.000000000","message":"Yep, fixed.","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"dea2e34b289f6f01ae7a05505e35053364a6352f","unresolved":false,"context_lines":[{"line_number":44,"context_line":"    \"\"\"Mixin class to extend subnet with service type attribute\"\"\""},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"    def _extend_subnet_service_type(self, subnet_res, subnet_db):"},{"line_number":47,"context_line":"        subnet_res[\u0027service_type\u0027] \u003d [service_type[\u0027service_type\u0027] for"},{"line_number":48,"context_line":"                                      service_type in"},{"line_number":49,"context_line":"                                      subnet_db[\u0027service_type\u0027]]"},{"line_number":50,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_8cc0322c","line":47,"in_reply_to":"bacf61ea_e107411a","updated":"2016-07-28 22:24:09.000000000","message":"Yes, bad oversight on my part (for one).","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"e883f1ad363ee1afafb380fe86c4d9b49cc8d1e8","unresolved":false,"context_lines":[{"line_number":36,"context_line":"                             nullable\u003dFalse)"},{"line_number":37,"context_line":"    subnet \u003d orm.relationship(models_v2.Subnet,"},{"line_number":38,"context_line":"                              backref\u003dorm.backref(\u0027service_types\u0027,"},{"line_number":39,"context_line":"                                                  lazy\u003d\u0027joined\u0027,"},{"line_number":40,"context_line":"                                                  uselist\u003dTrue))"},{"line_number":41,"context_line":"    __table_args__ \u003d ("},{"line_number":42,"context_line":"        sa.PrimaryKeyConstraint(\u0027subnet_id\u0027, \u0027service_type\u0027),"}],"source_content_type":"text/x-python","patch_set":14,"id":"bacf61ea_1003ef77","line":39,"updated":"2016-08-02 20:11:30.000000000","message":"I think this might need:\n\ncascade\u003d\u0027all, delete-orphan\u0027\n\nhttp://docs.sqlalchemy.org/en/latest/orm/cascades.html","commit_id":"b2922b713952d9de7d05cd20e6048e145a2869b1"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"f6ae63ffe1a47fc00092735ab7836766c4d17606","unresolved":false,"context_lines":[{"line_number":36,"context_line":"                             nullable\u003dFalse)"},{"line_number":37,"context_line":"    subnet \u003d orm.relationship(models_v2.Subnet,"},{"line_number":38,"context_line":"                              backref\u003dorm.backref(\u0027service_types\u0027,"},{"line_number":39,"context_line":"                                                  lazy\u003d\u0027joined\u0027,"},{"line_number":40,"context_line":"                                                  uselist\u003dTrue))"},{"line_number":41,"context_line":"    __table_args__ \u003d ("},{"line_number":42,"context_line":"        sa.PrimaryKeyConstraint(\u0027subnet_id\u0027, \u0027service_type\u0027),"}],"source_content_type":"text/x-python","patch_set":14,"id":"bacf61ea_995703ca","line":39,"in_reply_to":"bacf61ea_1003ef77","updated":"2016-08-03 02:15:37.000000000","message":"I verified this in a devstack test and it seems to work, i\u0027ll push out that change.","commit_id":"b2922b713952d9de7d05cd20e6048e145a2869b1"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7f0b4e9fb5bd51d09c6dbbfdaf1267d5e72a33f2","unresolved":false,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    subnet_id \u003d sa.Column(sa.String(36),"},{"line_number":31,"context_line":"                          sa.ForeignKey(\u0027subnets.id\u0027, ondelete\u003d\"CASCADE\"),"},{"line_number":32,"context_line":"                          nullable\u003dFalse)"},{"line_number":33,"context_line":"    # Service types must be valid device owners, therefore share max length"},{"line_number":34,"context_line":"    service_type \u003d sa.Column(sa.String("},{"line_number":35,"context_line":"                                length\u003dattributes.DEVICE_OWNER_MAX_LEN),"}],"source_content_type":"text/x-python","patch_set":15,"id":"9ad45d7e_bb3ab6cb","line":32,"updated":"2016-08-09 00:14:52.000000000","message":"nullable\u003dFalse can be omitted for both.","commit_id":"26434fbca793be3b62e8e3d94cde62b470395f19"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"bea289e129a274865bb1ca3c441c23d6f24672f7","unresolved":false,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    subnet_id \u003d sa.Column(sa.String(36),"},{"line_number":31,"context_line":"                          sa.ForeignKey(\u0027subnets.id\u0027, ondelete\u003d\"CASCADE\"),"},{"line_number":32,"context_line":"                          nullable\u003dFalse)"},{"line_number":33,"context_line":"    # Service types must be valid device owners, therefore share max length"},{"line_number":34,"context_line":"    service_type \u003d sa.Column(sa.String("},{"line_number":35,"context_line":"                                length\u003dattributes.DEVICE_OWNER_MAX_LEN),"}],"source_content_type":"text/x-python","patch_set":15,"id":"9ad45d7e_b8e625b2","line":32,"in_reply_to":"9ad45d7e_bb3ab6cb","updated":"2016-08-09 12:32:04.000000000","message":"Done","commit_id":"26434fbca793be3b62e8e3d94cde62b470395f19"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7f0b4e9fb5bd51d09c6dbbfdaf1267d5e72a33f2","unresolved":false,"context_lines":[{"line_number":51,"context_line":"    def _extend_subnet_service_types(self, subnet_res, subnet_db):"},{"line_number":52,"context_line":"        subnet_res[\u0027service_types\u0027] \u003d [service_type[\u0027service_type\u0027] for"},{"line_number":53,"context_line":"                                       service_type in"},{"line_number":54,"context_line":"                                       subnet_db[\u0027service_types\u0027]]"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    common_db_mixin.CommonDbMixin.register_dict_extend_funcs("},{"line_number":57,"context_line":"        attributes.SUBNETS, [_extend_subnet_service_types])"}],"source_content_type":"text/x-python","patch_set":15,"id":"9ad45d7e_bb41163c","line":54,"updated":"2016-08-09 00:14:52.000000000","message":"Thinking of a post upgrade scenario: are you sure this is going to work for existing subnets? E.g. prior to this extension being enabled? Is it possible that subnet_db has no key service_types or that subnet_db[\u0027service_types\u0027] is None?","commit_id":"26434fbca793be3b62e8e3d94cde62b470395f19"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"f45f6b49cea233e08ade8616214df5bc79eed9bb","unresolved":false,"context_lines":[{"line_number":51,"context_line":"    def _extend_subnet_service_types(self, subnet_res, subnet_db):"},{"line_number":52,"context_line":"        subnet_res[\u0027service_types\u0027] \u003d [service_type[\u0027service_type\u0027] for"},{"line_number":53,"context_line":"                                       service_type in"},{"line_number":54,"context_line":"                                       subnet_db[\u0027service_types\u0027]]"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    common_db_mixin.CommonDbMixin.register_dict_extend_funcs("},{"line_number":57,"context_line":"        attributes.SUBNETS, [_extend_subnet_service_types])"}],"source_content_type":"text/x-python","patch_set":15,"id":"9ad45d7e_7bc6624a","line":54,"in_reply_to":"9ad45d7e_18165155","updated":"2016-08-09 12:42:30.000000000","message":"I meant of course that we\u0027d use subnet_db.get(\u0027service_types\u0027, []) to protect ourselves for both missing keys, and None values.","commit_id":"26434fbca793be3b62e8e3d94cde62b470395f19"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"bea289e129a274865bb1ca3c441c23d6f24672f7","unresolved":false,"context_lines":[{"line_number":51,"context_line":"    def _extend_subnet_service_types(self, subnet_res, subnet_db):"},{"line_number":52,"context_line":"        subnet_res[\u0027service_types\u0027] \u003d [service_type[\u0027service_type\u0027] for"},{"line_number":53,"context_line":"                                       service_type in"},{"line_number":54,"context_line":"                                       subnet_db[\u0027service_types\u0027]]"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    common_db_mixin.CommonDbMixin.register_dict_extend_funcs("},{"line_number":57,"context_line":"        attributes.SUBNETS, [_extend_subnet_service_types])"}],"source_content_type":"text/x-python","patch_set":15,"id":"9ad45d7e_18165155","line":54,"in_reply_to":"9ad45d7e_bb41163c","updated":"2016-08-09 12:32:04.000000000","message":"Yes, I think that\u0027s something I missed. If we use subnet_db.get(\u0027service_types\u0027) instead then we\u0027ll be protected from subnets missing the key. None will then result in an empty list, which is fine. Thanks,","commit_id":"26434fbca793be3b62e8e3d94cde62b470395f19"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"b478d240e793995ac04002d0cd5177b8c4bb839e","unresolved":false,"context_lines":[{"line_number":49,"context_line":"    def _extend_subnet_service_types(self, subnet_res, subnet_db):"},{"line_number":50,"context_line":"        subnet_res[\u0027service_types\u0027] \u003d [service_type[\u0027service_type\u0027] for"},{"line_number":51,"context_line":"                                       service_type in"},{"line_number":52,"context_line":"                                       subnet_db.get(\u0027service_types\u0027)]"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    common_db_mixin.CommonDbMixin.register_dict_extend_funcs("},{"line_number":55,"context_line":"        attributes.SUBNETS, [_extend_subnet_service_types])"}],"source_content_type":"text/x-python","patch_set":16,"id":"9ad45d7e_5b59defd","line":52,"range":{"start_line":52,"start_character":0,"end_line":52,"end_character":70},"updated":"2016-08-09 12:38:18.000000000","message":"Missing the [] for default value here, my bad. Fix incoming.","commit_id":"85d656bfd09467781ac1183702cb5e2a23e2dfac"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"2c4f96f2c236db07030c9ef2313618b1443443e2","unresolved":false,"context_lines":[{"line_number":49,"context_line":"    def _extend_subnet_service_types(self, subnet_res, subnet_db):"},{"line_number":50,"context_line":"        subnet_res[\u0027service_types\u0027] \u003d [service_type[\u0027service_type\u0027] for"},{"line_number":51,"context_line":"                                       service_type in"},{"line_number":52,"context_line":"                                       subnet_db.get(\u0027service_types\u0027)]"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    common_db_mixin.CommonDbMixin.register_dict_extend_funcs("},{"line_number":55,"context_line":"        attributes.SUBNETS, [_extend_subnet_service_types])"}],"source_content_type":"text/x-python","patch_set":16,"id":"9ad45d7e_aa2fdd3c","line":52,"updated":"2016-08-09 14:59:32.000000000","message":"calling .get should not be necessary. The relationship should always be there because of the L35","commit_id":"85d656bfd09467781ac1183702cb5e2a23e2dfac"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"e683889347b4f42257a75ab3eee766a3d86c2968","unresolved":false,"context_lines":[{"line_number":48,"context_line":""},{"line_number":49,"context_line":"    def _extend_subnet_service_types(self, subnet_res, subnet_db):"},{"line_number":50,"context_line":"        subnet_res[\u0027service_types\u0027] \u003d [service_type[\u0027service_type\u0027] for"},{"line_number":51,"context_line":"                                       service_type in"},{"line_number":52,"context_line":"                                       subnet_db.get(\u0027service_types\u0027, [])]"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    common_db_mixin.CommonDbMixin.register_dict_extend_funcs("}],"source_content_type":"text/x-python","patch_set":17,"id":"9ad45d7e_eab715f3","line":51,"updated":"2016-08-09 15:00:48.000000000","message":"sorry, i was too slow before you uploaded. just do subnet_db.service_types here. The relationship will always be there if this module is loaded.","commit_id":"26664737a5fbd5b58904ba81372376fb306ea732"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"5e093f3fea25a56482de0b4ecd1aba3f64c43efd","unresolved":false,"context_lines":[{"line_number":48,"context_line":""},{"line_number":49,"context_line":"    def _extend_subnet_service_types(self, subnet_res, subnet_db):"},{"line_number":50,"context_line":"        subnet_res[\u0027service_types\u0027] \u003d [service_type[\u0027service_type\u0027] for"},{"line_number":51,"context_line":"                                       service_type in"},{"line_number":52,"context_line":"                                       subnet_db.get(\u0027service_types\u0027, [])]"},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    common_db_mixin.CommonDbMixin.register_dict_extend_funcs("}],"source_content_type":"text/x-python","patch_set":17,"id":"9ad45d7e_908b281e","line":51,"in_reply_to":"9ad45d7e_eab715f3","updated":"2016-08-09 15:31:27.000000000","message":"Done","commit_id":"26664737a5fbd5b58904ba81372376fb306ea732"}],"neutron/extensions/subnet_service_types.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"11c103751d779d343d5f7965ec51063890da9606","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"EXTENDED_ATTRIBUTES_2_0 \u003d {"},{"line_number":19,"context_line":"    attributes.SUBNETS: {"},{"line_number":20,"context_line":"        \u0027service_type\u0027: {\u0027allow_post\u0027: True,"},{"line_number":21,"context_line":"                         \u0027allow_put\u0027: True,"},{"line_number":22,"context_line":"                         \u0027default\u0027: None,"},{"line_number":23,"context_line":"                         \u0027is_visible\u0027: True, },"}],"source_content_type":"text/x-python","patch_set":1,"id":"1aa78d24_4e9d3809","line":20,"range":{"start_line":20,"start_character":9,"end_line":20,"end_character":21},"updated":"2016-07-06 20:35:49.000000000","message":"This needs at least some validation I think.","commit_id":"8aebc1e7ead2c17270ce301c7f35a4f67aee3042"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"af91fe95276994688f4104c0ebf39663bc96f870","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"EXTENDED_ATTRIBUTES_2_0 \u003d {"},{"line_number":19,"context_line":"    attributes.SUBNETS: {"},{"line_number":20,"context_line":"        \u0027service_type\u0027: {\u0027allow_post\u0027: True,"},{"line_number":21,"context_line":"                         \u0027allow_put\u0027: True,"},{"line_number":22,"context_line":"                         \u0027default\u0027: None,"},{"line_number":23,"context_line":"                         \u0027is_visible\u0027: True, },"}],"source_content_type":"text/x-python","patch_set":1,"id":"1aa78d24_c8881225","line":20,"range":{"start_line":20,"start_character":9,"end_line":20,"end_character":21},"in_reply_to":"1aa78d24_4e9d3809","updated":"2016-07-12 10:37:28.000000000","message":"Done","commit_id":"8aebc1e7ead2c17270ce301c7f35a4f67aee3042"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"a423f4d23f24e85fafff187b5e548d7d933e9506","unresolved":false,"context_lines":[{"line_number":41,"context_line":"                raise InvalidSubnetServiceType(service_type\u003dservice_type)"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"validators.validators[\u0027type:validate_subnet_service_types\u0027] \u003d ("},{"line_number":45,"context_line":"    _validate_subnet_service_types)"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"dada55a8_4b659a43","line":44,"updated":"2016-07-19 00:34:48.000000000","message":"please add a TODO so that we can clean this up once [1] is released.\n\n[1] https://review.openstack.org/#/c/324090","commit_id":"2c36c20e4c818b660176efba810cbf438ba1e879"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"383ba0d5a90f23826816ad27399f0e434cde0d87","unresolved":false,"context_lines":[{"line_number":41,"context_line":"                raise InvalidSubnetServiceType(service_type\u003dservice_type)"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"validators.validators[\u0027type:validate_subnet_service_types\u0027] \u003d ("},{"line_number":45,"context_line":"    _validate_subnet_service_types)"},{"line_number":46,"context_line":""},{"line_number":47,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"dada55a8_3fa4d1a4","line":44,"in_reply_to":"dada55a8_4b659a43","updated":"2016-07-19 11:14:54.000000000","message":"Done","commit_id":"2c36c20e4c818b660176efba810cbf438ba1e879"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"a423f4d23f24e85fafff187b5e548d7d933e9506","unresolved":false,"context_lines":[{"line_number":49,"context_line":"    attributes.SUBNETS: {"},{"line_number":50,"context_line":"        \u0027service_type\u0027: {\u0027allow_post\u0027: True,"},{"line_number":51,"context_line":"                         \u0027allow_put\u0027: True,"},{"line_number":52,"context_line":"                         \u0027default\u0027: None,"},{"line_number":53,"context_line":"                         \u0027validate\u0027: {\u0027type:validate_subnet_service_types\u0027:"},{"line_number":54,"context_line":"                                      None},"},{"line_number":55,"context_line":"                         \u0027is_visible\u0027: True, },"}],"source_content_type":"text/x-python","patch_set":6,"id":"dada55a8_0b4c42c6","line":52,"updated":"2016-07-19 00:34:48.000000000","message":"shouldn\u0027t this be ATTR_NOT_SPECIFIED?","commit_id":"2c36c20e4c818b660176efba810cbf438ba1e879"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"383ba0d5a90f23826816ad27399f0e434cde0d87","unresolved":false,"context_lines":[{"line_number":49,"context_line":"    attributes.SUBNETS: {"},{"line_number":50,"context_line":"        \u0027service_type\u0027: {\u0027allow_post\u0027: True,"},{"line_number":51,"context_line":"                         \u0027allow_put\u0027: True,"},{"line_number":52,"context_line":"                         \u0027default\u0027: None,"},{"line_number":53,"context_line":"                         \u0027validate\u0027: {\u0027type:validate_subnet_service_types\u0027:"},{"line_number":54,"context_line":"                                      None},"},{"line_number":55,"context_line":"                         \u0027is_visible\u0027: True, },"}],"source_content_type":"text/x-python","patch_set":6,"id":"dada55a8_9f3a1d97","line":52,"in_reply_to":"dada55a8_0b4c42c6","updated":"2016-07-19 11:14:54.000000000","message":"Done","commit_id":"2c36c20e4c818b660176efba810cbf438ba1e879"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"0443d77a35b18a421dd0a25b5d18fe28062f8a80","unresolved":false,"context_lines":[{"line_number":28,"context_line":""},{"line_number":29,"context_line":"def _validate_subnet_service_types(service_types, valid_values\u003dNone):"},{"line_number":30,"context_line":"    if service_types:"},{"line_number":31,"context_line":"        if not isinstance(service_types, list):"},{"line_number":32,"context_line":"            raise webob.exc.HTTPBadRequest("},{"line_number":33,"context_line":"                _(\"Subnet service type must be a list.\"))"},{"line_number":34,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"dada55a8_ee572500","line":31,"updated":"2016-07-19 11:33:25.000000000","message":"in general a validator does not raise an exception, it returns a string if there is an error","commit_id":"b733ccd6b5619f69668563a39204eb66a2ab02a3"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"e86e07d5d9880d3fed1a60e618a054c0dad7e45b","unresolved":false,"context_lines":[{"line_number":28,"context_line":""},{"line_number":29,"context_line":"def _validate_subnet_service_types(service_types, valid_values\u003dNone):"},{"line_number":30,"context_line":"    if service_types:"},{"line_number":31,"context_line":"        if not isinstance(service_types, list):"},{"line_number":32,"context_line":"            raise webob.exc.HTTPBadRequest("},{"line_number":33,"context_line":"                _(\"Subnet service type must be a list.\"))"},{"line_number":34,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"dada55a8_442c3102","line":31,"in_reply_to":"dada55a8_2e03fd43","updated":"2016-07-19 23:05:49.000000000","message":"It looks to me like the validator needs to ultimately raise an exception but some validators return a message which is then converted to an exception [1].\n\n [1] https://github.com/openstack/neutron/blob/122a971656671f92927d44ddd3725cca74b4e0bb/neutron/extensions/allowedaddresspairs.py#L67-L69","commit_id":"b733ccd6b5619f69668563a39204eb66a2ab02a3"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"11664c1f7e82659b67d96ec77b4b1c374536a436","unresolved":false,"context_lines":[{"line_number":28,"context_line":""},{"line_number":29,"context_line":"def _validate_subnet_service_types(service_types, valid_values\u003dNone):"},{"line_number":30,"context_line":"    if service_types:"},{"line_number":31,"context_line":"        if not isinstance(service_types, list):"},{"line_number":32,"context_line":"            raise webob.exc.HTTPBadRequest("},{"line_number":33,"context_line":"                _(\"Subnet service type must be a list.\"))"},{"line_number":34,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"dada55a8_2e03fd43","line":31,"in_reply_to":"dada55a8_ee572500","updated":"2016-07-19 11:49:00.000000000","message":"A cursory glance through neutron/extensions yields more examples of exceptions than strings. Is there a relevant hacking rule here?","commit_id":"b733ccd6b5619f69668563a39204eb66a2ab02a3"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"0443d77a35b18a421dd0a25b5d18fe28062f8a80","unresolved":false,"context_lines":[{"line_number":38,"context_line":"        for service_type in service_types:"},{"line_number":39,"context_line":"            # Stricter validation required?"},{"line_number":40,"context_line":"            if not service_type.startswith(prefixes):"},{"line_number":41,"context_line":"                raise InvalidSubnetServiceType(service_type\u003dservice_type)"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"# TODO(john-davidge): Update after the move to private validators"}],"source_content_type":"text/x-python","patch_set":7,"id":"dada55a8_8e77899e","line":41,"updated":"2016-07-19 11:33:25.000000000","message":"ditto","commit_id":"b733ccd6b5619f69668563a39204eb66a2ab02a3"},{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"671f32363a902788f3ce09b53f927d12e85a3cc9","unresolved":false,"context_lines":[{"line_number":35,"context_line":"        prefixes \u003d constants.DEVICE_OWNER_PREFIXES"},{"line_number":36,"context_line":"        prefixes +\u003d (constants.DEVICE_OWNER_COMPUTE_PREFIX,)"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"        for service_type in service_types:"},{"line_number":39,"context_line":"            # Stricter validation required?"},{"line_number":40,"context_line":"            if not service_type.startswith(prefixes):"},{"line_number":41,"context_line":"                raise InvalidSubnetServiceType(service_type\u003dservice_type)"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_6df891e6","line":38,"updated":"2016-07-28 04:16:47.000000000","message":"I\u0027m not sure if we should validate at all. A service plugin that uses different prefixes may also want to make use of this feature.\n\nIf we do want to validate, can you make this a list in the module so service plugins can register their own prefixes as necessary?","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"dce97d6e567d77f9fabf2cae3ce45fd4aed08ca1","unresolved":false,"context_lines":[{"line_number":35,"context_line":"        prefixes \u003d constants.DEVICE_OWNER_PREFIXES"},{"line_number":36,"context_line":"        prefixes +\u003d (constants.DEVICE_OWNER_COMPUTE_PREFIX,)"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"        for service_type in service_types:"},{"line_number":39,"context_line":"            # Stricter validation required?"},{"line_number":40,"context_line":"            if not service_type.startswith(prefixes):"},{"line_number":41,"context_line":"                raise InvalidSubnetServiceType(service_type\u003dservice_type)"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_0c045e15","line":38,"in_reply_to":"bacf61ea_6df891e6","updated":"2016-07-28 14:42:18.000000000","message":"I think some validation is desired to ensure that only useful values are given. I see your point about service plugins, I\u0027ll make this modifiable. Thanks.","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"498b1040a41a27ce694a206d75d434a908e7e3a9","unresolved":false,"context_lines":[{"line_number":36,"context_line":"        prefixes +\u003d (constants.DEVICE_OWNER_COMPUTE_PREFIX,)"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"        for service_type in service_types:"},{"line_number":39,"context_line":"            # Stricter validation required?"},{"line_number":40,"context_line":"            if not service_type.startswith(prefixes):"},{"line_number":41,"context_line":"                raise InvalidSubnetServiceType(service_type\u003dservice_type)"},{"line_number":42,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_c4c1a130","line":39,"updated":"2016-07-27 22:27:37.000000000","message":"I think we should remove this comment","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"dce97d6e567d77f9fabf2cae3ce45fd4aed08ca1","unresolved":false,"context_lines":[{"line_number":36,"context_line":"        prefixes +\u003d (constants.DEVICE_OWNER_COMPUTE_PREFIX,)"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"        for service_type in service_types:"},{"line_number":39,"context_line":"            # Stricter validation required?"},{"line_number":40,"context_line":"            if not service_type.startswith(prefixes):"},{"line_number":41,"context_line":"                raise InvalidSubnetServiceType(service_type\u003dservice_type)"},{"line_number":42,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_4cc136e4","line":39,"in_reply_to":"bacf61ea_c4c1a130","updated":"2016-07-28 14:42:18.000000000","message":"Done. It seems the debate is whether this is already too strict anyway!","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"233ef0644b07089f058ef6db58a79970cfb20dbc","unresolved":false,"context_lines":[{"line_number":41,"context_line":"                raise InvalidSubnetServiceType(service_type\u003dservice_type)"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"# TODO(john-davidge): Update after the move to private validators"},{"line_number":45,"context_line":"validators.validators[\u0027type:validate_subnet_service_types\u0027] \u003d ("},{"line_number":46,"context_line":"    _validate_subnet_service_types)"},{"line_number":47,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_dc35c2da","line":44,"updated":"2016-07-28 18:24:43.000000000","message":"As soon as [1] merges you can do this cleanly.\n\n[1] https://review.openstack.org/#/c/348468/\n[2] https://github.com/openstack/neutron-lib/blob/master/neutron_lib/api/validators.py#L646","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"233ef0644b07089f058ef6db58a79970cfb20dbc","unresolved":false,"context_lines":[{"line_number":48,"context_line":""},{"line_number":49,"context_line":"EXTENDED_ATTRIBUTES_2_0 \u003d {"},{"line_number":50,"context_line":"    attributes.SUBNETS: {"},{"line_number":51,"context_line":"        \u0027service_type\u0027: {\u0027allow_post\u0027: True,"},{"line_number":52,"context_line":"                         \u0027allow_put\u0027: True,"},{"line_number":53,"context_line":"                         \u0027default\u0027: constants.ATTR_NOT_SPECIFIED,"},{"line_number":54,"context_line":"                         \u0027validate\u0027: {\u0027type:validate_subnet_service_types\u0027:"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_7c3db6ba","line":51,"updated":"2016-07-28 18:24:43.000000000","message":"Am I the only one finding the singular for a plural resource problematic and inexpressive?","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"62e03ae9f6bf5a636aeda5731ab00d8f334a1f4f","unresolved":false,"context_lines":[{"line_number":48,"context_line":""},{"line_number":49,"context_line":"EXTENDED_ATTRIBUTES_2_0 \u003d {"},{"line_number":50,"context_line":"    attributes.SUBNETS: {"},{"line_number":51,"context_line":"        \u0027service_type\u0027: {\u0027allow_post\u0027: True,"},{"line_number":52,"context_line":"                         \u0027allow_put\u0027: True,"},{"line_number":53,"context_line":"                         \u0027default\u0027: constants.ATTR_NOT_SPECIFIED,"},{"line_number":54,"context_line":"                         \u0027validate\u0027: {\u0027type:validate_subnet_service_types\u0027:"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_38bb2a29","line":51,"in_reply_to":"bacf61ea_7c3db6ba","updated":"2016-07-29 11:00:35.000000000","message":"Done","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"233ef0644b07089f058ef6db58a79970cfb20dbc","unresolved":false,"context_lines":[{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    @classmethod"},{"line_number":69,"context_line":"    def get_alias(cls):"},{"line_number":70,"context_line":"        return \"subnet-service-types\""},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    @classmethod"},{"line_number":73,"context_line":"    def get_description(cls):"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_bc5e0e10","line":70,"updated":"2016-07-28 18:24:43.000000000","message":"typeS but then you extend with type? /me sad.","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"62e03ae9f6bf5a636aeda5731ab00d8f334a1f4f","unresolved":false,"context_lines":[{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    @classmethod"},{"line_number":69,"context_line":"    def get_alias(cls):"},{"line_number":70,"context_line":"        return \"subnet-service-types\""},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    @classmethod"},{"line_number":73,"context_line":"    def get_description(cls):"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_38920a9f","line":70,"in_reply_to":"bacf61ea_6c55fe4e","updated":"2016-07-29 11:00:35.000000000","message":"/me has fixed it :)","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"10693d1d375b957fd4e2dfa3e9c5483717b35698","unresolved":false,"context_lines":[{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    @classmethod"},{"line_number":69,"context_line":"    def get_alias(cls):"},{"line_number":70,"context_line":"        return \"subnet-service-types\""},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    @classmethod"},{"line_number":73,"context_line":"    def get_description(cls):"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_6c55fe4e","line":70,"in_reply_to":"bacf61ea_6cd21e02","updated":"2016-07-28 22:26:17.000000000","message":"/me happy bunny","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"dea2e34b289f6f01ae7a05505e35053364a6352f","unresolved":false,"context_lines":[{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    @classmethod"},{"line_number":69,"context_line":"    def get_alias(cls):"},{"line_number":70,"context_line":"        return \"subnet-service-types\""},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    @classmethod"},{"line_number":73,"context_line":"    def get_description(cls):"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_6cd21e02","line":70,"in_reply_to":"bacf61ea_bc5e0e10","updated":"2016-07-28 22:24:09.000000000","message":"Don\u0027t be sad, we\u0027ll fix it for you.","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"81c1401d1c64e2221b78f7cb19d4a8f51072dc53","unresolved":false,"context_lines":[{"line_number":1,"context_line":"#"},{"line_number":2,"context_line":"#    Licensed under the Apache License, Version 2.0 (the \"License\"); you may"},{"line_number":3,"context_line":"#    not use this file except in compliance with the License. You may obtain"},{"line_number":4,"context_line":"#    a copy of the License at"}],"source_content_type":"text/x-python","patch_set":14,"id":"bacf61ea_8134fe49","line":1,"updated":"2016-08-01 16:54:14.000000000","message":"nit: unneeded line","commit_id":"b2922b713952d9de7d05cd20e6048e145a2869b1"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"7f0b4e9fb5bd51d09c6dbbfdaf1267d5e72a33f2","unresolved":false,"context_lines":[{"line_number":46,"context_line":""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"# TODO(john-davidge): Update after the move to private validators"},{"line_number":49,"context_line":"validators.validators[\u0027type:validate_subnet_service_types\u0027] \u003d ("},{"line_number":50,"context_line":"    _validate_subnet_service_types)"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"9ad45d7e_7bcafea2","line":49,"updated":"2016-08-09 00:14:52.000000000","message":"Let\u0027s use [1] already.\n\n[1] https://github.com/openstack/neutron-lib/blob/master/neutron_lib/api/validators.py#L646","commit_id":"26434fbca793be3b62e8e3d94cde62b470395f19"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"bea289e129a274865bb1ca3c441c23d6f24672f7","unresolved":false,"context_lines":[{"line_number":46,"context_line":""},{"line_number":47,"context_line":""},{"line_number":48,"context_line":"# TODO(john-davidge): Update after the move to private validators"},{"line_number":49,"context_line":"validators.validators[\u0027type:validate_subnet_service_types\u0027] \u003d ("},{"line_number":50,"context_line":"    _validate_subnet_service_types)"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"9ad45d7e_e3d3d25d","line":49,"in_reply_to":"9ad45d7e_7bcafea2","updated":"2016-08-09 12:32:04.000000000","message":"Done","commit_id":"26434fbca793be3b62e8e3d94cde62b470395f19"}],"neutron/plugins/ml2/plugin.py":[{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"b486fa6826f4bc85d08ae5b6d67d01c45048afa6","unresolved":false,"context_lines":[{"line_number":105,"context_line":"                vlantransparent_db.Vlantransparent_db_mixin,"},{"line_number":106,"context_line":"                extradhcpopt_db.ExtraDhcpOptMixin,"},{"line_number":107,"context_line":"                address_scope_db.AddressScopeDbMixin,"},{"line_number":108,"context_line":"                service_type_db.SubnetServiceTypeMixin):"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    \"\"\"Implement the Neutron L2 abstractions using modules."},{"line_number":111,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_a1773fd3","line":108,"updated":"2016-07-27 22:26:15.000000000","message":"Over the past few months I\u0027ve seen several times Armando express eloquently his unhappiness with developers adding mixins here. So I think it is better to discuss it among ourselves before he chimes in. Adding the \u0027service_type\u0027 attribute to subnets is very similar to what we did adding \u0027dns_name\u0027 to ports or \u0027dns_domain\u0027 to networks. We implemented both cases with a Ml2 extension: http://git.openstack.org/cgit/openstack/neutron/tree/neutron/plugins/ml2/extensions/dns_integration.py. The original implementation used a mixin like the one used here. It didn\u0027t take much effort to convert it to a ML2 extension and we completely avoided the mixin approach. I\u0027ll be more than happy to help if needed","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"7a6d1c5b468a137a23062636c7861876e08b21d3","unresolved":false,"context_lines":[{"line_number":105,"context_line":"                vlantransparent_db.Vlantransparent_db_mixin,"},{"line_number":106,"context_line":"                extradhcpopt_db.ExtraDhcpOptMixin,"},{"line_number":107,"context_line":"                address_scope_db.AddressScopeDbMixin,"},{"line_number":108,"context_line":"                service_type_db.SubnetServiceTypeMixin):"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    \"\"\"Implement the Neutron L2 abstractions using modules."},{"line_number":111,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_aa47eb49","line":108,"in_reply_to":"bacf61ea_13b8f6e4","updated":"2016-07-28 15:46:48.000000000","message":"I had the same thought. But, I convinced myself that this was pretty minimal. I hope armax isn\u0027t disappointed with me.  :(","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"62e03ae9f6bf5a636aeda5731ab00d8f334a1f4f","unresolved":false,"context_lines":[{"line_number":105,"context_line":"                vlantransparent_db.Vlantransparent_db_mixin,"},{"line_number":106,"context_line":"                extradhcpopt_db.ExtraDhcpOptMixin,"},{"line_number":107,"context_line":"                address_scope_db.AddressScopeDbMixin,"},{"line_number":108,"context_line":"                service_type_db.SubnetServiceTypeMixin):"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    \"\"\"Implement the Neutron L2 abstractions using modules."},{"line_number":111,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_782da245","line":108,"in_reply_to":"bacf61ea_212cb901","updated":"2016-07-29 11:00:35.000000000","message":"Thanks Armando, that was my line of thinking too. In that case I\u0027ll keep the approach as it is.","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"dce97d6e567d77f9fabf2cae3ce45fd4aed08ca1","unresolved":false,"context_lines":[{"line_number":105,"context_line":"                vlantransparent_db.Vlantransparent_db_mixin,"},{"line_number":106,"context_line":"                extradhcpopt_db.ExtraDhcpOptMixin,"},{"line_number":107,"context_line":"                address_scope_db.AddressScopeDbMixin,"},{"line_number":108,"context_line":"                service_type_db.SubnetServiceTypeMixin):"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    \"\"\"Implement the Neutron L2 abstractions using modules."},{"line_number":111,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_13b8f6e4","line":108,"in_reply_to":"bacf61ea_a1773fd3","updated":"2016-07-28 14:42:18.000000000","message":"Reading Armando\u0027s comments on [1] it seems to me that his objection to this type of approach was quite specific to that particular case (\"I was surprised by the way DNS was so heavily tied with ML2, the mixin business etc...\"). He\u0027s reviewed this patch once already and didn\u0027t have any objections at the time, but obviously I\u0027d like to be sure he\u0027s happy with this approach before it merges. I\u0027ll reach out to him for comment. It would be nice to avoid a major re-write at this late stage.\n\n[1] https://review.openstack.org/#/c/212213/","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":748,"name":"Armando Migliaccio","email":"armamig@gmail.com","username":"armando-migliaccio"},"change_message_id":"233ef0644b07089f058ef6db58a79970cfb20dbc","unresolved":false,"context_lines":[{"line_number":105,"context_line":"                vlantransparent_db.Vlantransparent_db_mixin,"},{"line_number":106,"context_line":"                extradhcpopt_db.ExtraDhcpOptMixin,"},{"line_number":107,"context_line":"                address_scope_db.AddressScopeDbMixin,"},{"line_number":108,"context_line":"                service_type_db.SubnetServiceTypeMixin):"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    \"\"\"Implement the Neutron L2 abstractions using modules."},{"line_number":111,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_212cb901","line":108,"in_reply_to":"bacf61ea_aa47eb49","updated":"2016-07-28 18:24:43.000000000","message":"Even though this perpetrates the bad practice of using mixins, this feels like a minor evil as subnets are already a construct that ML2 understands, being part of the core plugin interface. We did the same for AZ support as that is bolted on top of the agent extensions, so I am ok-ish with this :)\n\nDNS was in a slightly different position, but as Miguel pointed out this can be achieved nonetheless via an ML2 extension driver. The drawback to that is that the driver must be explicitly configured by the operator, unless we can come up with a way to load a set of default extensions.","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"b486fa6826f4bc85d08ae5b6d67d01c45048afa6","unresolved":false,"context_lines":[{"line_number":134,"context_line":"                                    \"availability_zone\","},{"line_number":135,"context_line":"                                    \"network_availability_zone\","},{"line_number":136,"context_line":"                                    \"default-subnetpools\","},{"line_number":137,"context_line":"                                    \"subnet-service-types\"]"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":"    @property"},{"line_number":140,"context_line":"    def supported_extension_aliases(self):"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_84c16983","line":137,"updated":"2016-07-27 22:26:15.000000000","message":"Please see my comment to line 108. If we decide to use a ML2 extension, we need to remove this","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"}],"neutron/tests/etc/policy.json":[{"author":{"_account_id":7787,"name":"Kevin Benton","email":"kevin@benton.pub","username":"blak111"},"change_message_id":"671f32363a902788f3ce09b53f927d12e85a3cc9","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"    \"create_subnet\": \"rule:admin_or_network_owner\","},{"line_number":19,"context_line":"    \"create_subnet:segment_id\": \"rule:admin_only\","},{"line_number":20,"context_line":"    \"create_subnet:service_type\": \"rule:admin_only\","},{"line_number":21,"context_line":"    \"get_subnet\": \"rule:admin_or_owner or rule:shared\","},{"line_number":22,"context_line":"    \"get_subnet:segment_id\": \"rule:admin_only\","},{"line_number":23,"context_line":"    \"update_subnet\": \"rule:admin_or_network_owner\","}],"source_content_type":"application/json","patch_set":12,"id":"bacf61ea_0de8f50f","line":20,"updated":"2016-07-28 04:16:47.000000000","message":"does this actually need to be admin only? It just sets preferences for IP allocation so I don\u0027t see a reason that this shouldn\u0027t be made available to users that may want to slice up their IPs based on the availability zones their instances land in.","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"62e03ae9f6bf5a636aeda5731ab00d8f334a1f4f","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"    \"create_subnet\": \"rule:admin_or_network_owner\","},{"line_number":19,"context_line":"    \"create_subnet:segment_id\": \"rule:admin_only\","},{"line_number":20,"context_line":"    \"create_subnet:service_type\": \"rule:admin_only\","},{"line_number":21,"context_line":"    \"get_subnet\": \"rule:admin_or_owner or rule:shared\","},{"line_number":22,"context_line":"    \"get_subnet:segment_id\": \"rule:admin_only\","},{"line_number":23,"context_line":"    \"update_subnet\": \"rule:admin_or_network_owner\","}],"source_content_type":"application/json","patch_set":12,"id":"bacf61ea_6105ee78","line":20,"in_reply_to":"bacf61ea_0de8f50f","updated":"2016-07-29 11:00:35.000000000","message":"The service_type is only intended to affect IP allocation on external networks, therefore it\u0027s admin only. If/when that changes then it might make sense to open it up to all users.","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"}],"neutron/tests/unit/extensions/test_subnet_service_types.py":[{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"6dcfa3f4c5db40465e159d99c3b57bdfa21be120","unresolved":false,"context_lines":[{"line_number":97,"context_line":"                                      cidr\u003dcidr)"},{"line_number":98,"context_line":"            subnet \u003d self.deserialize(\u0027json\u0027, res)"},{"line_number":99,"context_line":"            subnet \u003d subnet[\u0027subnet\u0027]"},{"line_number":100,"context_line":"            self.assertFalse(subnet[\u0027service_type\u0027])"}],"source_content_type":"text/x-python","patch_set":5,"id":"1aa78d24_00dc2173","line":100,"updated":"2016-07-14 16:47:01.000000000","message":"There are no tests for update.","commit_id":"dd2520b84d037d97d005319c3c2f16d6dd9a94b1"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"61158d387282c13a9182365736d57743e53c3c42","unresolved":false,"context_lines":[{"line_number":97,"context_line":"                                      cidr\u003dcidr)"},{"line_number":98,"context_line":"            subnet \u003d self.deserialize(\u0027json\u0027, res)"},{"line_number":99,"context_line":"            subnet \u003d subnet[\u0027subnet\u0027]"},{"line_number":100,"context_line":"            self.assertFalse(subnet[\u0027service_type\u0027])"}],"source_content_type":"text/x-python","patch_set":5,"id":"fad55976_64c5665a","line":100,"in_reply_to":"1aa78d24_00dc2173","updated":"2016-07-15 15:50:02.000000000","message":"True, I\u0027ll go ahead and add some. Please see my reply to your comment in ipam_backend_mixin.py about update behavior.","commit_id":"dd2520b84d037d97d005319c3c2f16d6dd9a94b1"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"e86e07d5d9880d3fed1a60e618a054c0dad7e45b","unresolved":false,"context_lines":[{"line_number":124,"context_line":"                                      tenant_id\u003dnetwork[\u0027tenant_id\u0027],"},{"line_number":125,"context_line":"                                      cidr\u003dcidr)"},{"line_number":126,"context_line":"            subnet \u003d self.deserialize(\u0027json\u0027, res)[\u0027subnet\u0027]"},{"line_number":127,"context_line":"            # Update it with a single service type"},{"line_number":128,"context_line":"            self._update_subnet(subnet, service_type)"},{"line_number":129,"context_line":"            # Update it with a second service type"},{"line_number":130,"context_line":"            service_type.append(\u0027compute:bar\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"dada55a8_24993d20","line":127,"updated":"2016-07-19 23:05:49.000000000","message":"In general, a test should test one thing and be done.  I\u0027m not a big fan of these single tests that progressively test multiple things. Could you split this out into multiple discrete tests?","commit_id":"b733ccd6b5619f69668563a39204eb66a2ab02a3"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"9454edf998abcd34a9bd5890b9af11c3acdecf61","unresolved":false,"context_lines":[{"line_number":124,"context_line":"                                      tenant_id\u003dnetwork[\u0027tenant_id\u0027],"},{"line_number":125,"context_line":"                                      cidr\u003dcidr)"},{"line_number":126,"context_line":"            subnet \u003d self.deserialize(\u0027json\u0027, res)[\u0027subnet\u0027]"},{"line_number":127,"context_line":"            # Update it with a single service type"},{"line_number":128,"context_line":"            self._update_subnet(subnet, service_type)"},{"line_number":129,"context_line":"            # Update it with a second service type"},{"line_number":130,"context_line":"            service_type.append(\u0027compute:bar\u0027)"}],"source_content_type":"text/x-python","patch_set":7,"id":"dada55a8_c71f6d06","line":127,"in_reply_to":"dada55a8_24993d20","updated":"2016-07-20 11:43:19.000000000","message":"Done","commit_id":"b733ccd6b5619f69668563a39204eb66a2ab02a3"},{"author":{"_account_id":22220,"name":"Anindita Das","email":"anindita.das@utexas.edu","username":"anudas"},"change_message_id":"43b3f03606261ddb8f575786fe82043d06333165","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        with self.network() as network:"},{"line_number":57,"context_line":"            network \u003d network[\u0027network\u0027]"},{"line_number":58,"context_line":"            cidr \u003d \u002710.0.0.0/8\u0027"},{"line_number":59,"context_line":"            return self._create_subnet(self.fmt,"},{"line_number":60,"context_line":"                                       net_id\u003dnetwork[\u0027id\u0027],"},{"line_number":61,"context_line":"                                       tenant_id\u003dnetwork[\u0027tenant_id\u0027],"},{"line_number":62,"context_line":"                                       cidr\u003dcidr,"}],"source_content_type":"text/x-python","patch_set":8,"id":"dada55a8_dfd51a43","line":59,"updated":"2016-07-21 18:50:24.000000000","message":"How does this test ensure that create_subnet is being performed by admin_only as specified in the policy.json?","commit_id":"e7c34dba119afe4ccc8fdff56ac8e0b130601977"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"7fbdf612a06dff71aca4b58c67b485611ea8c493","unresolved":false,"context_lines":[{"line_number":56,"context_line":"        with self.network() as network:"},{"line_number":57,"context_line":"            network \u003d network[\u0027network\u0027]"},{"line_number":58,"context_line":"            cidr \u003d \u002710.0.0.0/8\u0027"},{"line_number":59,"context_line":"            return self._create_subnet(self.fmt,"},{"line_number":60,"context_line":"                                       net_id\u003dnetwork[\u0027id\u0027],"},{"line_number":61,"context_line":"                                       tenant_id\u003dnetwork[\u0027tenant_id\u0027],"},{"line_number":62,"context_line":"                                       cidr\u003dcidr,"}],"source_content_type":"text/x-python","patch_set":8,"id":"dada55a8_33b20595","line":59,"in_reply_to":"dada55a8_dfd51a43","updated":"2016-07-26 12:27:13.000000000","message":"I believe this level of coverage would require a tempest api test. I could add one to this patchset if it seems necessary, or I could make it part of the next patch. Alternatively it could be covered in the OSC tests.","commit_id":"e7c34dba119afe4ccc8fdff56ac8e0b130601977"},{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"a7de66c48d2334b638bb6ed2e38cdc044dcc788f","unresolved":false,"context_lines":[{"line_number":55,"context_line":"    def _create_subnet_with_service_type(self, service_type):"},{"line_number":56,"context_line":"        with self.network() as network:"},{"line_number":57,"context_line":"            network \u003d network[\u0027network\u0027]"},{"line_number":58,"context_line":"            cidr \u003d \u002710.0.0.0/8\u0027"},{"line_number":59,"context_line":"            return self._create_subnet(self.fmt,"},{"line_number":60,"context_line":"                                       net_id\u003dnetwork[\u0027id\u0027],"},{"line_number":61,"context_line":"                                       tenant_id\u003dnetwork[\u0027tenant_id\u0027],"}],"source_content_type":"text/x-python","patch_set":11,"id":"bacf61ea_34d1cece","line":58,"updated":"2016-07-27 21:25:37.000000000","message":"We should add an IPv6 version as well, I\u0027ll send an update.","commit_id":"7b019a744aac7e9516be5a054cb205e1a69036b2"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"b486fa6826f4bc85d08ae5b6d67d01c45048afa6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"bacf61ea_a4958d41","updated":"2016-07-27 22:26:15.000000000","message":"If we decide to follow the ML2 extension approach, I think we should move this tests module to neutron.tests.plugins.ml2.extensions","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":4694,"name":"Miguel Lavalle","email":"miguel@mlavalle.com","username":"minsel"},"change_message_id":"b486fa6826f4bc85d08ae5b6d67d01c45048afa6","unresolved":false,"context_lines":[{"line_number":41,"context_line":""},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"class SubnetServiceTypesExtensionTestCase("},{"line_number":44,"context_line":"         test_db_base_plugin_v2.NeutronDbPluginV2TestCase):"},{"line_number":45,"context_line":"    \"\"\"Test API extension subnet_service_types attributes."},{"line_number":46,"context_line":"    \"\"\""},{"line_number":47,"context_line":"    CIDR \u003d \u002710.0.0.0/8\u0027"}],"source_content_type":"text/x-python","patch_set":12,"id":"bacf61ea_441491f4","line":44,"range":{"start_line":44,"start_character":9,"end_line":44,"end_character":57},"updated":"2016-07-27 22:26:15.000000000","message":"If we decide to take the Ml2 extension approach, we will need to inherit from neutron.tests.unit.plugins.ml2.test_plugin.Ml2PluginV2TestCase","commit_id":"a3cc182309d94ce16738ba87d71dbe101af943fb"},{"author":{"_account_id":7448,"name":"Carl Baldwin","email":"carl@ecbaldwin.net","username":"carl-baldwin"},"change_message_id":"1a58cd8fe26ff0cdf870decf9420a980990c25be","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        super(SubnetServiceTypesExtensionTestCase,"},{"line_number":55,"context_line":"              self).setUp(plugin\u003dplugin, ext_mgr\u003dext_mgr)"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    def _create_subnet_with_service_types(self, service_types):"},{"line_number":58,"context_line":"        with self.network() as network:"},{"line_number":59,"context_line":"            network \u003d network[\u0027network\u0027]"},{"line_number":60,"context_line":"            return self._create_subnet(self.fmt,"}],"source_content_type":"text/x-python","patch_set":15,"id":"9ad45d7e_ccc1ba30","line":57,"updated":"2016-08-08 21:57:42.000000000","message":"I just noticed that this method is nearly identical to the next one. I think if service_types were an optional argument (i.e. service_types\u003dNone) then you\u0027d only need one.","commit_id":"26434fbca793be3b62e8e3d94cde62b470395f19"},{"author":{"_account_id":6635,"name":"John Davidge","email":"john.davidge@rackspace.com","username":"john-davidge"},"change_message_id":"bea289e129a274865bb1ca3c441c23d6f24672f7","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        super(SubnetServiceTypesExtensionTestCase,"},{"line_number":55,"context_line":"              self).setUp(plugin\u003dplugin, ext_mgr\u003dext_mgr)"},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    def _create_subnet_with_service_types(self, service_types):"},{"line_number":58,"context_line":"        with self.network() as network:"},{"line_number":59,"context_line":"            network \u003d network[\u0027network\u0027]"},{"line_number":60,"context_line":"            return self._create_subnet(self.fmt,"}],"source_content_type":"text/x-python","patch_set":15,"id":"9ad45d7e_89b15592","line":57,"in_reply_to":"9ad45d7e_ccc1ba30","updated":"2016-08-09 12:32:04.000000000","message":"Done","commit_id":"26434fbca793be3b62e8e3d94cde62b470395f19"}]}