)]}'
{"keystone/catalog/core.py":[{"author":{"_account_id":9101,"name":"wanghong","email":"reviewwanghong@126.com","username":"wanghong"},"change_message_id":"3499b6c9d8d09a6e1082d1d9bf1576a1879ff4c8","unresolved":false,"context_lines":[{"line_number":228,"context_line":"        try:"},{"line_number":229,"context_line":"            v3_catalog \u003d self.driver.get_v3_catalog(user_id, project_id,"},{"line_number":230,"context_line":"                                                    metadata\u003dmetadata)"},{"line_number":231,"context_line":"            self.get_federation_service_providers(v3_catalog)"},{"line_number":232,"context_line":"            return v3_catalog"},{"line_number":233,"context_line":"        except exception.NotFound:"},{"line_number":234,"context_line":"            raise exception.NotFound(\u0027Catalog not found for user and tenant\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"da86d52c_87cdfe88","line":231,"updated":"2015-02-17 06:49:39.000000000","message":"I think we should change get_federation_service_providers to add_federation_service_providers or something.","commit_id":"ba0addc4a0edbbd62dcc93c5814b194a091d0264"},{"author":{"_account_id":8978,"name":"Marek Denis","email":"marek.denis+openstack@gmail.com","username":"marek-denis"},"change_message_id":"c36235dc94cfe4af11048bb141929d063f487e09","unresolved":false,"context_lines":[{"line_number":228,"context_line":"        try:"},{"line_number":229,"context_line":"            v3_catalog \u003d self.driver.get_v3_catalog(user_id, project_id,"},{"line_number":230,"context_line":"                                                    metadata\u003dmetadata)"},{"line_number":231,"context_line":"            self.get_federation_service_providers(v3_catalog)"},{"line_number":232,"context_line":"            return v3_catalog"},{"line_number":233,"context_line":"        except exception.NotFound:"},{"line_number":234,"context_line":"            raise exception.NotFound(\u0027Catalog not found for user and tenant\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"da86d52c_f6f02d68","line":231,"in_reply_to":"da86d52c_87cdfe88","updated":"2015-02-17 08:44:01.000000000","message":"Done","commit_id":"ba0addc4a0edbbd62dcc93c5814b194a091d0264"},{"author":{"_account_id":9101,"name":"wanghong","email":"reviewwanghong@126.com","username":"wanghong"},"change_message_id":"3499b6c9d8d09a6e1082d1d9bf1576a1879ff4c8","unresolved":false,"context_lines":[{"line_number":242,"context_line":"        :type v3_catalog: list of dictionaries"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"        :returns: service_provider_ref: List of service_provider objects"},{"line_number":245,"context_line":"        :rtype: dict"},{"line_number":246,"context_line":""},{"line_number":247,"context_line":"        \"\"\""},{"line_number":248,"context_line":"        federation_api \u003d getattr(self, \u0027federation_api\u0027, None)"}],"source_content_type":"text/x-python","patch_set":8,"id":"da86d52c_67ae4a9a","line":245,"updated":"2015-02-17 06:49:39.000000000","message":"It seams that this method returns None.","commit_id":"ba0addc4a0edbbd62dcc93c5814b194a091d0264"},{"author":{"_account_id":8978,"name":"Marek Denis","email":"marek.denis+openstack@gmail.com","username":"marek-denis"},"change_message_id":"c36235dc94cfe4af11048bb141929d063f487e09","unresolved":false,"context_lines":[{"line_number":242,"context_line":"        :type v3_catalog: list of dictionaries"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"        :returns: service_provider_ref: List of service_provider objects"},{"line_number":245,"context_line":"        :rtype: dict"},{"line_number":246,"context_line":""},{"line_number":247,"context_line":"        \"\"\""},{"line_number":248,"context_line":"        federation_api \u003d getattr(self, \u0027federation_api\u0027, None)"}],"source_content_type":"text/x-python","patch_set":8,"id":"da86d52c_5601f937","line":245,"in_reply_to":"da86d52c_67ae4a9a","updated":"2015-02-17 08:44:01.000000000","message":"Done","commit_id":"ba0addc4a0edbbd62dcc93c5814b194a091d0264"},{"author":{"_account_id":13478,"name":"Boris Bobrov","email":"b.bobrov@sap.com","username":"bbobrov"},"change_message_id":"f1b8fa54398f7dab881d4ef164b526a91ddec152","unresolved":false,"context_lines":[{"line_number":231,"context_line":"            self.add_federation_service_providers(v3_catalog)"},{"line_number":232,"context_line":"            return v3_catalog"},{"line_number":233,"context_line":"        except exception.NotFound:"},{"line_number":234,"context_line":"            raise exception.NotFound(\u0027Catalog not found for user and tenant\u0027)"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"    def add_federation_service_providers(self, v3_catalog):"},{"line_number":237,"context_line":"        \"\"\"Add service providers to Service Catalog dict"}],"source_content_type":"text/x-python","patch_set":9,"id":"da86d52c_5e39073e","line":234,"updated":"2015-02-17 13:43:23.000000000","message":"Why do you raise the same exception here? You could re-raise or just remove the try-except block.","commit_id":"6dd41c4156453b2b6245874fa1767863f02b4b11"},{"author":{"_account_id":8978,"name":"Marek Denis","email":"marek.denis+openstack@gmail.com","username":"marek-denis"},"change_message_id":"2e888f52183692d8ee35fd6782737b438f8eba38","unresolved":false,"context_lines":[{"line_number":231,"context_line":"            self.add_federation_service_providers(v3_catalog)"},{"line_number":232,"context_line":"            return v3_catalog"},{"line_number":233,"context_line":"        except exception.NotFound:"},{"line_number":234,"context_line":"            raise exception.NotFound(\u0027Catalog not found for user and tenant\u0027)"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"    def add_federation_service_providers(self, v3_catalog):"},{"line_number":237,"context_line":"        \"\"\"Add service providers to Service Catalog dict"}],"source_content_type":"text/x-python","patch_set":9,"id":"da86d52c_581f52b7","line":234,"in_reply_to":"da86d52c_5e39073e","updated":"2015-02-17 17:32:08.000000000","message":"I removed it here.\nIn fact there are some other places (line 222ish) where this occurs, so another patch could be tossed.","commit_id":"6dd41c4156453b2b6245874fa1767863f02b4b11"},{"author":{"_account_id":6482,"name":"Steve Martinelli","email":"s.martinelli@gmail.com","username":"stevemar"},"change_message_id":"862f4a915f2b7fc9c836fa8d3523c6f424a5744d","unresolved":false,"context_lines":[{"line_number":240,"context_line":"        :type v3_catalog: list of dictionaries"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"        \"\"\""},{"line_number":243,"context_line":"        federation_api \u003d getattr(self, \u0027federation_api\u0027, None)"},{"line_number":244,"context_line":"        if federation_api is None:"},{"line_number":245,"context_line":"            return"},{"line_number":246,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"da86d52c_5ce72b12","line":243,"updated":"2015-02-18 08:03:47.000000000","message":"might want to revisit the \u0027if self.federation_api is None:\u0027 code you initially had, now that federation is always spun up.","commit_id":"33e4857510d0b4c621ed46918da8babccdbbc88b"},{"author":{"_account_id":1916,"name":"Guang Yee","email":"gyee@suse.com","username":"guang-yee"},"change_message_id":"40f60532688c505a1f348fd4c694e5db68b1a708","unresolved":false,"context_lines":[{"line_number":230,"context_line":"        self.add_federation_service_providers(v3_catalog)"},{"line_number":231,"context_line":"        return v3_catalog"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    def add_federation_service_providers(self, v3_catalog):"},{"line_number":234,"context_line":"        \"\"\"Add service providers to Service Catalog dict"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"        Modify v3_catalog, and add service_providers entry."}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_1d3cb12e","line":233,"updated":"2015-02-18 19:51:52.000000000","message":"As mentioned in the API spec, I think these are unnecessary. Why treat federated service providers differently from any other \"services\", whether they are IaaS or services running on top of the cloud? They are all \"services\".","commit_id":"06347c6bd855e90a5d43c48efd07fc7f30539432"},{"author":{"_account_id":1916,"name":"Guang Yee","email":"gyee@suse.com","username":"guang-yee"},"change_message_id":"6383d0690a8888358968d834df0deb40e95e2ec0","unresolved":false,"context_lines":[{"line_number":230,"context_line":"        self.add_federation_service_providers(v3_catalog)"},{"line_number":231,"context_line":"        return v3_catalog"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    def add_federation_service_providers(self, v3_catalog):"},{"line_number":234,"context_line":"        \"\"\"Add service providers to Service Catalog dict"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"        Modify v3_catalog, and add service_providers entry."}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_ef01de39","line":233,"in_reply_to":"da86d52c_18659fe0","updated":"2015-02-18 21:04:41.000000000","message":"I don\u0027t think so. On the contrary, I think having a dichotomy of service providers and openstack services introduce unnecessary complexity. Fundamentally, an endpoint is just an url and a set of attributes (i.e. interface, name, etc) which uniquely characterize that url. Service catalog can, and should, contain endpoints which user are allowed to access. And that includes IaaS or services running on top of the cloud (i.e. DBaaS). Service provider is just another endpoint providing some service, which in the case it happens to be a Keystone endpoint provider federation APIs.\n\nClient does endpoint lookup using endpoint filtering mechanism, which is just a generic set of attribute-value pair. If we have the dichotomy of service providers in the service catalog, which is structurally different from an endpoint, that would also means we need to introduce additional and unnecessary service catalog logic on the client sides as well.","commit_id":"06347c6bd855e90a5d43c48efd07fc7f30539432"},{"author":{"_account_id":6482,"name":"Steve Martinelli","email":"s.martinelli@gmail.com","username":"stevemar"},"change_message_id":"8e16a9bec7e32d36acb83b77f78d48f0e5d3457c","unresolved":false,"context_lines":[{"line_number":230,"context_line":"        self.add_federation_service_providers(v3_catalog)"},{"line_number":231,"context_line":"        return v3_catalog"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    def add_federation_service_providers(self, v3_catalog):"},{"line_number":234,"context_line":"        \"\"\"Add service providers to Service Catalog dict"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"        Modify v3_catalog, and add service_providers entry."}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_18659fe0","line":233,"in_reply_to":"da86d52c_1d3cb12e","updated":"2015-02-18 20:40:44.000000000","message":"the \u0027services\u0027 in the endpoint are specific to that openstack deployment, and follow a fairly rigid endpoint based structure. the \u0027service providers\u0027 we are listing here are not specific to the openstack deployment that the user has authenticated against. so I think adding them to the usual list of endpoints/services would muddle things, these are different, and as such should be treated differently.","commit_id":"06347c6bd855e90a5d43c48efd07fc7f30539432"},{"author":{"_account_id":8978,"name":"Marek Denis","email":"marek.denis+openstack@gmail.com","username":"marek-denis"},"change_message_id":"123eb94e8dfb4492b1fe2794dab71a68258aa2ff","unresolved":false,"context_lines":[{"line_number":230,"context_line":"        self.add_federation_service_providers(v3_catalog)"},{"line_number":231,"context_line":"        return v3_catalog"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    def add_federation_service_providers(self, v3_catalog):"},{"line_number":234,"context_line":"        \"\"\"Add service providers to Service Catalog dict"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"        Modify v3_catalog, and add service_providers entry."}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_c242310e","line":233,"in_reply_to":"da86d52c_ef01de39","updated":"2015-02-18 21:17:43.000000000","message":"so, how do you imagine proper Service Catalog should look like?","commit_id":"06347c6bd855e90a5d43c48efd07fc7f30539432"},{"author":{"_account_id":6482,"name":"Steve Martinelli","email":"s.martinelli@gmail.com","username":"stevemar"},"change_message_id":"8e16a9bec7e32d36acb83b77f78d48f0e5d3457c","unresolved":false,"context_lines":[{"line_number":240,"context_line":"        :type v3_catalog: list of dictionaries"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"        \"\"\""},{"line_number":243,"context_line":"        federation_api \u003d getattr(self, \u0027federation_api\u0027, None)"},{"line_number":244,"context_line":"        if federation_api is None:"},{"line_number":245,"context_line":"            return"},{"line_number":246,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_2c730417","line":243,"updated":"2015-02-18 20:40:44.000000000","message":"still wondering if theres a better way to do this","commit_id":"06347c6bd855e90a5d43c48efd07fc7f30539432"},{"author":{"_account_id":6482,"name":"Steve Martinelli","email":"s.martinelli@gmail.com","username":"stevemar"},"change_message_id":"6242e2b7e3bad4ab52f21d5798bcc8a14e20dfb6","unresolved":false,"context_lines":[{"line_number":80,"context_line":"    return result"},{"line_number":81,"context_line":""},{"line_number":82,"context_line":""},{"line_number":83,"context_line":"@dependency.optional(\u0027federation_api\u0027)"},{"line_number":84,"context_line":"@dependency.provider(\u0027catalog_api\u0027)"},{"line_number":85,"context_line":"class Manager(manager.Manager):"},{"line_number":86,"context_line":"    \"\"\"Default pivot point for the Catalog backend."}],"source_content_type":"text/x-python","patch_set":12,"id":"da86d52c_0ccff37f","line":83,"updated":"2015-02-19 18:53:34.000000000","message":"make this dependency.requires(\u0027federation_api\u0027)","commit_id":"e4afa76be56235a17c060796eddaef8a55158f75"},{"author":{"_account_id":6482,"name":"Steve Martinelli","email":"s.martinelli@gmail.com","username":"stevemar"},"change_message_id":"6242e2b7e3bad4ab52f21d5798bcc8a14e20dfb6","unresolved":false,"context_lines":[{"line_number":240,"context_line":"        :type v3_catalog: list of dictionaries"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"        \"\"\""},{"line_number":243,"context_line":"        federation_api \u003d getattr(self, \u0027federation_api\u0027, None)"},{"line_number":244,"context_line":"        if federation_api is None:"},{"line_number":245,"context_line":"            return"},{"line_number":246,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"da86d52c_ccb15b02","line":243,"updated":"2015-02-19 18:53:34.000000000","message":"you should be able to just federation_api now, it should always be loaded","commit_id":"e4afa76be56235a17c060796eddaef8a55158f75"}],"keystone/contrib/federation/core.py":[{"author":{"_account_id":9101,"name":"wanghong","email":"reviewwanghong@126.com","username":"wanghong"},"change_message_id":"3499b6c9d8d09a6e1082d1d9bf1576a1879ff4c8","unresolved":false,"context_lines":[{"line_number":296,"context_line":"        \"\"\"List enabled service providers for Service Catalog"},{"line_number":297,"context_line":""},{"line_number":298,"context_line":"        Service Provider objects contain three attributes: ``id``,"},{"line_number":299,"context_line":"        ``auth_url``, ``sp_url``. For example"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"        returns: list of dictionaries instantiating enabled service providers"},{"line_number":302,"context_line":"        rtype: list of dicts"}],"source_content_type":"text/x-python","patch_set":8,"id":"da86d52c_4799466d","line":299,"updated":"2015-02-17 06:49:39.000000000","message":"For example? I don\u0027t see the example:)","commit_id":"ba0addc4a0edbbd62dcc93c5814b194a091d0264"},{"author":{"_account_id":8978,"name":"Marek Denis","email":"marek.denis+openstack@gmail.com","username":"marek-denis"},"change_message_id":"c36235dc94cfe4af11048bb141929d063f487e09","unresolved":false,"context_lines":[{"line_number":296,"context_line":"        \"\"\"List enabled service providers for Service Catalog"},{"line_number":297,"context_line":""},{"line_number":298,"context_line":"        Service Provider objects contain three attributes: ``id``,"},{"line_number":299,"context_line":"        ``auth_url``, ``sp_url``. For example"},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"        returns: list of dictionaries instantiating enabled service providers"},{"line_number":302,"context_line":"        rtype: list of dicts"}],"source_content_type":"text/x-python","patch_set":8,"id":"da86d52c_d69409ea","line":299,"in_reply_to":"da86d52c_4799466d","updated":"2015-02-17 08:44:01.000000000","message":"hah, good catch! :-)","commit_id":"ba0addc4a0edbbd62dcc93c5814b194a091d0264"},{"author":{"_account_id":13478,"name":"Boris Bobrov","email":"b.bobrov@sap.com","username":"bbobrov"},"change_message_id":"f1b8fa54398f7dab881d4ef164b526a91ddec152","unresolved":false,"context_lines":[{"line_number":298,"context_line":"        Service Provider objects contain three attributes: ``id``,"},{"line_number":299,"context_line":"        ``auth_url``, ``sp_url``."},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"        returns: list of dictionaries instantiating enabled service providers"},{"line_number":302,"context_line":"        rtype: list of dicts"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":9,"id":"da86d52c_dec97726","line":301,"updated":"2015-02-17 13:43:23.000000000","message":"should not it be :returns: and :rtype:?","commit_id":"6dd41c4156453b2b6245874fa1767863f02b4b11"},{"author":{"_account_id":8978,"name":"Marek Denis","email":"marek.denis+openstack@gmail.com","username":"marek-denis"},"change_message_id":"2e888f52183692d8ee35fd6782737b438f8eba38","unresolved":false,"context_lines":[{"line_number":298,"context_line":"        Service Provider objects contain three attributes: ``id``,"},{"line_number":299,"context_line":"        ``auth_url``, ``sp_url``."},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"        returns: list of dictionaries instantiating enabled service providers"},{"line_number":302,"context_line":"        rtype: list of dicts"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":9,"id":"da86d52c_58e4b2ab","line":301,"in_reply_to":"da86d52c_dec97726","updated":"2015-02-17 17:32:08.000000000","message":"very likely.","commit_id":"6dd41c4156453b2b6245874fa1767863f02b4b11"},{"author":{"_account_id":6482,"name":"Steve Martinelli","email":"s.martinelli@gmail.com","username":"stevemar"},"change_message_id":"862f4a915f2b7fc9c836fa8d3523c6f424a5744d","unresolved":false,"context_lines":[{"line_number":295,"context_line":"    def get_sc_service_providers(self):"},{"line_number":296,"context_line":"        \"\"\"List enabled service providers for Service Catalog"},{"line_number":297,"context_line":""},{"line_number":298,"context_line":"        Service Provider objects contain three attributes: ``id``,"},{"line_number":299,"context_line":"        ``auth_url``, ``sp_url``."},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"        :returns: list of dictionaries instantiating enabled service providers"}],"source_content_type":"text/x-python","patch_set":10,"id":"da86d52c_bc982776","line":298,"updated":"2015-02-18 08:03:47.000000000","message":"nit: Service Provider *in a catalog* contains...","commit_id":"33e4857510d0b4c621ed46918da8babccdbbc88b"},{"author":{"_account_id":8978,"name":"Marek Denis","email":"marek.denis+openstack@gmail.com","username":"marek-denis"},"change_message_id":"d49fa1a6730142b4f2b0ab47504fbe77b292ea03","unresolved":false,"context_lines":[{"line_number":295,"context_line":"    def get_sc_service_providers(self):"},{"line_number":296,"context_line":"        \"\"\"List enabled service providers for Service Catalog"},{"line_number":297,"context_line":""},{"line_number":298,"context_line":"        Service Provider objects contain three attributes: ``id``,"},{"line_number":299,"context_line":"        ``auth_url``, ``sp_url``."},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"        :returns: list of dictionaries instantiating enabled service providers"}],"source_content_type":"text/x-python","patch_set":10,"id":"da86d52c_6d9b4417","line":298,"in_reply_to":"da86d52c_bc982776","updated":"2015-02-18 16:46:18.000000000","message":"Done","commit_id":"33e4857510d0b4c621ed46918da8babccdbbc88b"},{"author":{"_account_id":6482,"name":"Steve Martinelli","email":"s.martinelli@gmail.com","username":"stevemar"},"change_message_id":"862f4a915f2b7fc9c836fa8d3523c6f424a5744d","unresolved":false,"context_lines":[{"line_number":298,"context_line":"        Service Provider objects contain three attributes: ``id``,"},{"line_number":299,"context_line":"        ``auth_url``, ``sp_url``."},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"        :returns: list of dictionaries instantiating enabled service providers"},{"line_number":302,"context_line":"        :rtype: list of dicts"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":10,"id":"da86d52c_3c84370a","line":301,"updated":"2015-02-18 08:03:47.000000000","message":"nit: list of dictionaries *with* enabled service providers?","commit_id":"33e4857510d0b4c621ed46918da8babccdbbc88b"},{"author":{"_account_id":8978,"name":"Marek Denis","email":"marek.denis+openstack@gmail.com","username":"marek-denis"},"change_message_id":"d49fa1a6730142b4f2b0ab47504fbe77b292ea03","unresolved":false,"context_lines":[{"line_number":298,"context_line":"        Service Provider objects contain three attributes: ``id``,"},{"line_number":299,"context_line":"        ``auth_url``, ``sp_url``."},{"line_number":300,"context_line":""},{"line_number":301,"context_line":"        :returns: list of dictionaries instantiating enabled service providers"},{"line_number":302,"context_line":"        :rtype: list of dicts"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":10,"id":"da86d52c_8da07843","line":301,"in_reply_to":"da86d52c_3c84370a","updated":"2015-02-18 16:46:18.000000000","message":"Done","commit_id":"33e4857510d0b4c621ed46918da8babccdbbc88b"}],"keystone/tests/unit/test_v3.py":[{"author":{"_account_id":9101,"name":"wanghong","email":"reviewwanghong@126.com","username":"wanghong"},"change_message_id":"3499b6c9d8d09a6e1082d1d9bf1576a1879ff4c8","unresolved":false,"context_lines":[{"line_number":794,"context_line":"        for service in entity:"},{"line_number":795,"context_line":"            for x in (\u0027endpoints\u0027, \u0027service_providers\u0027):"},{"line_number":796,"context_line":"                if x in service:"},{"line_number":797,"context_line":"                    validators[x](service)"},{"line_number":798,"context_line":""},{"line_number":799,"context_line":"    # region validation"},{"line_number":800,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"da86d52c_2715329b","line":797,"updated":"2015-02-17 06:49:39.000000000","message":"How about:\nif \u0027endpoints\u0027 in service:\n    validate_endpoints(service)\nif \u0027service_providers\u0027 in service:\n    validate_service_providers(service)\nI think it would be clearer?","commit_id":"ba0addc4a0edbbd62dcc93c5814b194a091d0264"},{"author":{"_account_id":8978,"name":"Marek Denis","email":"marek.denis+openstack@gmail.com","username":"marek-denis"},"change_message_id":"c36235dc94cfe4af11048bb141929d063f487e09","unresolved":false,"context_lines":[{"line_number":794,"context_line":"        for service in entity:"},{"line_number":795,"context_line":"            for x in (\u0027endpoints\u0027, \u0027service_providers\u0027):"},{"line_number":796,"context_line":"                if x in service:"},{"line_number":797,"context_line":"                    validators[x](service)"},{"line_number":798,"context_line":""},{"line_number":799,"context_line":"    # region validation"},{"line_number":800,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"da86d52c_f6be6d63","line":797,"in_reply_to":"da86d52c_2715329b","updated":"2015-02-17 08:44:01.000000000","message":"Readibility \u003e hackerness.\n(as much as i don\u0027t like \"if elif elif else\" chains)","commit_id":"ba0addc4a0edbbd62dcc93c5814b194a091d0264"},{"author":{"_account_id":13478,"name":"Boris Bobrov","email":"b.bobrov@sap.com","username":"bbobrov"},"change_message_id":"f1b8fa54398f7dab881d4ef164b526a91ddec152","unresolved":false,"context_lines":[{"line_number":778,"context_line":""},{"line_number":779,"context_line":"        def validate_service_providers(service_providers):"},{"line_number":780,"context_line":"            # make sure the list is not empty"},{"line_number":781,"context_line":"            self.assertTrue(len(service_providers))"},{"line_number":782,"context_line":"            for sp in service_providers:"},{"line_number":783,"context_line":"                self.assertIn(\u0027id\u0027, sp)"},{"line_number":784,"context_line":"                self.assertNotIn(\u0027enabled\u0027, sp)"}],"source_content_type":"text/x-python","patch_set":9,"id":"da86d52c_bebbe373","line":781,"updated":"2015-02-17 13:43:23.000000000","message":"Why not assertTrue(service_providers) or even \"assert service_providers\"?","commit_id":"6dd41c4156453b2b6245874fa1767863f02b4b11"},{"author":{"_account_id":8978,"name":"Marek Denis","email":"marek.denis+openstack@gmail.com","username":"marek-denis"},"change_message_id":"2e888f52183692d8ee35fd6782737b438f8eba38","unresolved":false,"context_lines":[{"line_number":778,"context_line":""},{"line_number":779,"context_line":"        def validate_service_providers(service_providers):"},{"line_number":780,"context_line":"            # make sure the list is not empty"},{"line_number":781,"context_line":"            self.assertTrue(len(service_providers))"},{"line_number":782,"context_line":"            for sp in service_providers:"},{"line_number":783,"context_line":"                self.assertIn(\u0027id\u0027, sp)"},{"line_number":784,"context_line":"                self.assertNotIn(\u0027enabled\u0027, sp)"}],"source_content_type":"text/x-python","patch_set":9,"id":"da86d52c_f8b80677","line":781,"in_reply_to":"da86d52c_bebbe373","updated":"2015-02-17 17:32:08.000000000","message":"Done","commit_id":"6dd41c4156453b2b6245874fa1767863f02b4b11"},{"author":{"_account_id":13478,"name":"Boris Bobrov","email":"b.bobrov@sap.com","username":"bbobrov"},"change_message_id":"f1b8fa54398f7dab881d4ef164b526a91ddec152","unresolved":false,"context_lines":[{"line_number":785,"context_line":"                self.assertNotIn(\u0027description\u0027, sp)"},{"line_number":786,"context_line":""},{"line_number":787,"context_line":"        self.assertIsInstance(entity, list)"},{"line_number":788,"context_line":"        self.assertTrue(len(entity) \u003e 0)"},{"line_number":789,"context_line":"        for service in entity:"},{"line_number":790,"context_line":"            if \u0027endpoints\u0027 in service:"},{"line_number":791,"context_line":"                validate_endpoints(service)"}],"source_content_type":"text/x-python","patch_set":9,"id":"da86d52c_1e706fdb","line":788,"updated":"2015-02-17 13:43:23.000000000","message":"same here","commit_id":"6dd41c4156453b2b6245874fa1767863f02b4b11"},{"author":{"_account_id":8978,"name":"Marek Denis","email":"marek.denis+openstack@gmail.com","username":"marek-denis"},"change_message_id":"2e888f52183692d8ee35fd6782737b438f8eba38","unresolved":false,"context_lines":[{"line_number":785,"context_line":"                self.assertNotIn(\u0027description\u0027, sp)"},{"line_number":786,"context_line":""},{"line_number":787,"context_line":"        self.assertIsInstance(entity, list)"},{"line_number":788,"context_line":"        self.assertTrue(len(entity) \u003e 0)"},{"line_number":789,"context_line":"        for service in entity:"},{"line_number":790,"context_line":"            if \u0027endpoints\u0027 in service:"},{"line_number":791,"context_line":"                validate_endpoints(service)"}],"source_content_type":"text/x-python","patch_set":9,"id":"da86d52c_18af9a2c","line":788,"in_reply_to":"da86d52c_1e706fdb","updated":"2015-02-17 17:32:08.000000000","message":"Done","commit_id":"6dd41c4156453b2b6245874fa1767863f02b4b11"},{"author":{"_account_id":6482,"name":"Steve Martinelli","email":"s.martinelli@gmail.com","username":"stevemar"},"change_message_id":"b04c36573de8581dfd9bfbfd4acb4f3596631cd2","unresolved":false,"context_lines":[{"line_number":778,"context_line":""},{"line_number":779,"context_line":"        def validate_service_providers(service_providers):"},{"line_number":780,"context_line":"            # make sure the list is not empty"},{"line_number":781,"context_line":"            self.assertTrue(service_providers)"},{"line_number":782,"context_line":"            for sp in service_providers:"},{"line_number":783,"context_line":"                self.assertIn(\u0027id\u0027, sp)"},{"line_number":784,"context_line":"                self.assertNotIn(\u0027enabled\u0027, sp)"}],"source_content_type":"text/x-python","patch_set":10,"id":"da86d52c_d0e3ff31","line":781,"updated":"2015-02-18 08:16:41.000000000","message":"i feel like this should also be:\n\n  self.assertTrue(len(service_providers) \u003e 0)","commit_id":"33e4857510d0b4c621ed46918da8babccdbbc88b"},{"author":{"_account_id":8978,"name":"Marek Denis","email":"marek.denis+openstack@gmail.com","username":"marek-denis"},"change_message_id":"d49fa1a6730142b4f2b0ab47504fbe77b292ea03","unresolved":false,"context_lines":[{"line_number":778,"context_line":""},{"line_number":779,"context_line":"        def validate_service_providers(service_providers):"},{"line_number":780,"context_line":"            # make sure the list is not empty"},{"line_number":781,"context_line":"            self.assertTrue(service_providers)"},{"line_number":782,"context_line":"            for sp in service_providers:"},{"line_number":783,"context_line":"                self.assertIn(\u0027id\u0027, sp)"},{"line_number":784,"context_line":"                self.assertNotIn(\u0027enabled\u0027, sp)"}],"source_content_type":"text/x-python","patch_set":10,"id":"da86d52c_0de00825","line":781,"in_reply_to":"da86d52c_d0c05f2d","updated":"2015-02-18 16:46:18.000000000","message":"I think both will have same effect: \n\nl1 \u003d []\nl2 \u003d [1,2,3,4]\n\nassert bool(l1) \u003d\u003d bool(len(l1) \u003e 0)\nassert bool(l2) \u003d\u003d bool(len(l2) \u003e 0)","commit_id":"33e4857510d0b4c621ed46918da8babccdbbc88b"},{"author":{"_account_id":13478,"name":"Boris Bobrov","email":"b.bobrov@sap.com","username":"bbobrov"},"change_message_id":"4ed21426995a9cc616c869f59be38279911c89ac","unresolved":false,"context_lines":[{"line_number":778,"context_line":""},{"line_number":779,"context_line":"        def validate_service_providers(service_providers):"},{"line_number":780,"context_line":"            # make sure the list is not empty"},{"line_number":781,"context_line":"            self.assertTrue(service_providers)"},{"line_number":782,"context_line":"            for sp in service_providers:"},{"line_number":783,"context_line":"                self.assertIn(\u0027id\u0027, sp)"},{"line_number":784,"context_line":"                self.assertNotIn(\u0027enabled\u0027, sp)"}],"source_content_type":"text/x-python","patch_set":10,"id":"da86d52c_d0c05f2d","line":781,"in_reply_to":"da86d52c_d0e3ff31","updated":"2015-02-18 08:26:51.000000000","message":"Why? :) I\u0027ve asked to change \"self.assertTrue(len(service_providers) \u003e 0)\" to current in the previous patchset.","commit_id":"33e4857510d0b4c621ed46918da8babccdbbc88b"},{"author":{"_account_id":6482,"name":"Steve Martinelli","email":"s.martinelli@gmail.com","username":"stevemar"},"change_message_id":"b04c36573de8581dfd9bfbfd4acb4f3596631cd2","unresolved":false,"context_lines":[{"line_number":780,"context_line":"            # make sure the list is not empty"},{"line_number":781,"context_line":"            self.assertTrue(service_providers)"},{"line_number":782,"context_line":"            for sp in service_providers:"},{"line_number":783,"context_line":"                self.assertIn(\u0027id\u0027, sp)"},{"line_number":784,"context_line":"                self.assertNotIn(\u0027enabled\u0027, sp)"},{"line_number":785,"context_line":"                self.assertNotIn(\u0027description\u0027, sp)"},{"line_number":786,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"da86d52c_b0c12b82","line":783,"updated":"2015-02-18 08:16:41.000000000","message":"also assert auth_url and sp_url","commit_id":"33e4857510d0b4c621ed46918da8babccdbbc88b"},{"author":{"_account_id":8978,"name":"Marek Denis","email":"marek.denis+openstack@gmail.com","username":"marek-denis"},"change_message_id":"d49fa1a6730142b4f2b0ab47504fbe77b292ea03","unresolved":false,"context_lines":[{"line_number":780,"context_line":"            # make sure the list is not empty"},{"line_number":781,"context_line":"            self.assertTrue(service_providers)"},{"line_number":782,"context_line":"            for sp in service_providers:"},{"line_number":783,"context_line":"                self.assertIn(\u0027id\u0027, sp)"},{"line_number":784,"context_line":"                self.assertNotIn(\u0027enabled\u0027, sp)"},{"line_number":785,"context_line":"                self.assertNotIn(\u0027description\u0027, sp)"},{"line_number":786,"context_line":""}],"source_content_type":"text/x-python","patch_set":10,"id":"da86d52c_0d4e4838","line":783,"in_reply_to":"da86d52c_b0c12b82","updated":"2015-02-18 16:46:18.000000000","message":"I am not sure we should requre that since both sp_url not auth_url are nullable.","commit_id":"33e4857510d0b4c621ed46918da8babccdbbc88b"},{"author":{"_account_id":6482,"name":"Steve Martinelli","email":"s.martinelli@gmail.com","username":"stevemar"},"change_message_id":"b04c36573de8581dfd9bfbfd4acb4f3596631cd2","unresolved":false,"context_lines":[{"line_number":785,"context_line":"                self.assertNotIn(\u0027description\u0027, sp)"},{"line_number":786,"context_line":""},{"line_number":787,"context_line":"        self.assertIsInstance(entity, list)"},{"line_number":788,"context_line":"        self.assertTrue(entity)"},{"line_number":789,"context_line":"        for service in entity:"},{"line_number":790,"context_line":"            if \u0027endpoints\u0027 in service:"},{"line_number":791,"context_line":"                validate_endpoints(service)"}],"source_content_type":"text/x-python","patch_set":10,"id":"da86d52c_9068a7bb","line":788,"updated":"2015-02-18 08:16:41.000000000","message":"you changed this, should be:\n\n  self.assertTrue(len(entity) \u003e 0)","commit_id":"33e4857510d0b4c621ed46918da8babccdbbc88b"},{"author":{"_account_id":6482,"name":"Steve Martinelli","email":"s.martinelli@gmail.com","username":"stevemar"},"change_message_id":"b04c36573de8581dfd9bfbfd4acb4f3596631cd2","unresolved":false,"context_lines":[{"line_number":789,"context_line":"        for service in entity:"},{"line_number":790,"context_line":"            if \u0027endpoints\u0027 in service:"},{"line_number":791,"context_line":"                validate_endpoints(service)"},{"line_number":792,"context_line":"            if \u0027service\u0027 in service:"},{"line_number":793,"context_line":"                validate_service_providers(service)"},{"line_number":794,"context_line":""},{"line_number":795,"context_line":"    # region validation"}],"source_content_type":"text/x-python","patch_set":10,"id":"da86d52c_f089e36f","line":792,"updated":"2015-02-18 08:16:41.000000000","message":"should this be \u0027if \u0027service_providers\u0027 in service:\u0027","commit_id":"33e4857510d0b4c621ed46918da8babccdbbc88b"},{"author":{"_account_id":8978,"name":"Marek Denis","email":"marek.denis+openstack@gmail.com","username":"marek-denis"},"change_message_id":"d49fa1a6730142b4f2b0ab47504fbe77b292ea03","unresolved":false,"context_lines":[{"line_number":789,"context_line":"        for service in entity:"},{"line_number":790,"context_line":"            if \u0027endpoints\u0027 in service:"},{"line_number":791,"context_line":"                validate_endpoints(service)"},{"line_number":792,"context_line":"            if \u0027service\u0027 in service:"},{"line_number":793,"context_line":"                validate_service_providers(service)"},{"line_number":794,"context_line":""},{"line_number":795,"context_line":"    # region validation"}],"source_content_type":"text/x-python","patch_set":10,"id":"da86d52c_925b3985","line":792,"in_reply_to":"da86d52c_f089e36f","updated":"2015-02-18 16:46:18.000000000","message":"my bad!","commit_id":"33e4857510d0b4c621ed46918da8babccdbbc88b"}],"keystone/tests/unit/test_v3_federation.py":[{"author":{"_account_id":13478,"name":"Boris Bobrov","email":"b.bobrov@sap.com","username":"bbobrov"},"change_message_id":"f1b8fa54398f7dab881d4ef164b526a91ddec152","unresolved":false,"context_lines":[{"line_number":2698,"context_line":"                                                  self.project[\u0027id\u0027])"},{"line_number":2699,"context_line":"        for entity in catalog:"},{"line_number":2700,"context_line":"            if \u0027service_providers\u0027 in entity:"},{"line_number":2701,"context_line":"                raise AttributeError(\u0027Expected Service Catalog not to have\u0027"},{"line_number":2702,"context_line":"                                     \u0027service_providers\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"da86d52c_c19a78c4","line":2701,"updated":"2015-02-17 13:43:23.000000000","message":"assertNotIn should be used here","commit_id":"6dd41c4156453b2b6245874fa1767863f02b4b11"},{"author":{"_account_id":8978,"name":"Marek Denis","email":"marek.denis+openstack@gmail.com","username":"marek-denis"},"change_message_id":"2e888f52183692d8ee35fd6782737b438f8eba38","unresolved":false,"context_lines":[{"line_number":2698,"context_line":"                                                  self.project[\u0027id\u0027])"},{"line_number":2699,"context_line":"        for entity in catalog:"},{"line_number":2700,"context_line":"            if \u0027service_providers\u0027 in entity:"},{"line_number":2701,"context_line":"                raise AttributeError(\u0027Expected Service Catalog not to have\u0027"},{"line_number":2702,"context_line":"                                     \u0027service_providers\u0027)"}],"source_content_type":"text/x-python","patch_set":9,"id":"da86d52c_7829769a","line":2701,"in_reply_to":"da86d52c_c19a78c4","updated":"2015-02-17 17:32:08.000000000","message":"Done","commit_id":"6dd41c4156453b2b6245874fa1767863f02b4b11"},{"author":{"_account_id":6482,"name":"Steve Martinelli","email":"s.martinelli@gmail.com","username":"stevemar"},"change_message_id":"b04c36573de8581dfd9bfbfd4acb4f3596631cd2","unresolved":false,"context_lines":[{"line_number":2629,"context_line":"        ref[\u0027id\u0027] \u003d id"},{"line_number":2630,"context_line":"        return ref"},{"line_number":2631,"context_line":""},{"line_number":2632,"context_line":"    def sp_ref(self):"},{"line_number":2633,"context_line":"        ref \u003d {"},{"line_number":2634,"context_line":"            \u0027auth_url\u0027: uuid.uuid4().hex,"},{"line_number":2635,"context_line":"            \u0027enabled\u0027: True,"}],"source_content_type":"text/x-python","patch_set":10,"id":"da86d52c_105f57e6","line":2632,"updated":"2015-02-18 08:16:41.000000000","message":"you defined this up top already","commit_id":"33e4857510d0b4c621ed46918da8babccdbbc88b"},{"author":{"_account_id":8978,"name":"Marek Denis","email":"marek.denis+openstack@gmail.com","username":"marek-denis"},"change_message_id":"d49fa1a6730142b4f2b0ab47504fbe77b292ea03","unresolved":false,"context_lines":[{"line_number":2629,"context_line":"        ref[\u0027id\u0027] \u003d id"},{"line_number":2630,"context_line":"        return ref"},{"line_number":2631,"context_line":""},{"line_number":2632,"context_line":"    def sp_ref(self):"},{"line_number":2633,"context_line":"        ref \u003d {"},{"line_number":2634,"context_line":"            \u0027auth_url\u0027: uuid.uuid4().hex,"},{"line_number":2635,"context_line":"            \u0027enabled\u0027: True,"}],"source_content_type":"text/x-python","patch_set":10,"id":"da86d52c_b2259d0b","line":2632,"in_reply_to":"da86d52c_105f57e6","updated":"2015-02-18 16:46:18.000000000","message":"yep, but (multi)inheriting the classes, adding someprivate classes to share one method makes me thing it will not increase code readibility or so.","commit_id":"33e4857510d0b4c621ed46918da8babccdbbc88b"},{"author":{"_account_id":6482,"name":"Steve Martinelli","email":"s.martinelli@gmail.com","username":"stevemar"},"change_message_id":"8e16a9bec7e32d36acb83b77f78d48f0e5d3457c","unresolved":false,"context_lines":[{"line_number":2629,"context_line":"        ref[\u0027id\u0027] \u003d id"},{"line_number":2630,"context_line":"        return ref"},{"line_number":2631,"context_line":""},{"line_number":2632,"context_line":"    def sp_ref(self):"},{"line_number":2633,"context_line":"        ref \u003d {"},{"line_number":2634,"context_line":"            \u0027auth_url\u0027: uuid.uuid4().hex,"},{"line_number":2635,"context_line":"            \u0027enabled\u0027: True,"}],"source_content_type":"text/x-python","patch_set":11,"id":"da86d52c_ec6c8c38","line":2632,"updated":"2015-02-18 20:40:44.000000000","message":"i\u0027m not happy about this, but we can clean it up another time","commit_id":"06347c6bd855e90a5d43c48efd07fc7f30539432"}]}