)]}'
{"keystone/assignment/core.py":[{"author":{"_account_id":5707,"name":"Henry Nash","email":"henryn@linux.vnet.ibm.com","username":"henry-nash"},"change_message_id":"2b52609bd77e3a3ba4d5be950a232cf6f5fef444","unresolved":false,"context_lines":[{"line_number":310,"context_line":"    def create_domain(self, domain_id, domain):"},{"line_number":311,"context_line":"        if (not self.identity_api.is_domain_aware() and"},{"line_number":312,"context_line":"            domain_id !\u003d CONF.identity.default_domain_id and"},{"line_number":313,"context_line":"                domain_id is not None):"},{"line_number":314,"context_line":"            raise exception.Forbidden()"},{"line_number":315,"context_line":"        domain.setdefault(\u0027enabled\u0027, True)"},{"line_number":316,"context_line":"        domain[\u0027enabled\u0027] \u003d clean.domain_enabled(domain[\u0027enabled\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_aa12f75f","line":313,"updated":"2014-08-26 14:14:59.000000000","message":"a couple of things:\n\n1) I don\u0027t think domain_id is allowed to be None here\n\n2) I think you also need to check if domain_specific_drivers are enabled, since it might be that although the default domain is not domain aware, other domains are supported via other domain specific config files\n\nPerhaps this whole test should sit are the identity manager layer (e.g. self.identity_api.multiple_domains_support() ) rather than encode the logic in a test in assignment.","commit_id":"cdd6f58fb4f4261a3fa5c76ee70bfbda449bf72b"},{"author":{"_account_id":12215,"name":"Alexey Miroshkin","email":"alexey.miroshkin@ru.ibm.com","username":"amirosh"},"change_message_id":"7501f15c3d402d3885d4805ac7b4ad14e53ff0ba","unresolved":false,"context_lines":[{"line_number":310,"context_line":"    def create_domain(self, domain_id, domain):"},{"line_number":311,"context_line":"        if (not self.identity_api.is_domain_aware() and"},{"line_number":312,"context_line":"            domain_id !\u003d CONF.identity.default_domain_id and"},{"line_number":313,"context_line":"                domain_id is not None):"},{"line_number":314,"context_line":"            raise exception.Forbidden()"},{"line_number":315,"context_line":"        domain.setdefault(\u0027enabled\u0027, True)"},{"line_number":316,"context_line":"        domain[\u0027enabled\u0027] \u003d clean.domain_enabled(domain[\u0027enabled\u0027])"}],"source_content_type":"text/x-python","patch_set":1,"id":"1abeadc6_41a3e54e","line":313,"in_reply_to":"1abeadc6_aa12f75f","updated":"2014-08-27 09:08:36.000000000","message":"Done","commit_id":"cdd6f58fb4f4261a3fa5c76ee70bfbda449bf72b"},{"author":{"_account_id":4,"name":"Dolph Mathews","email":"dolph.mathews@gmail.com","username":"dolph"},"change_message_id":"42f78a0086e7f90856057b4326b4cbc309f8076b","unresolved":false,"context_lines":[{"line_number":310,"context_line":"    def create_domain(self, domain_id, domain):"},{"line_number":311,"context_line":"        if (not self.identity_api.multiple_domains_support() and"},{"line_number":312,"context_line":"                domain_id !\u003d CONF.identity.default_domain_id):"},{"line_number":313,"context_line":"            raise exception.Forbidden()"},{"line_number":314,"context_line":"        domain.setdefault(\u0027enabled\u0027, True)"},{"line_number":315,"context_line":"        domain[\u0027enabled\u0027] \u003d clean.domain_enabled(domain[\u0027enabled\u0027])"},{"line_number":316,"context_line":"        ret \u003d self.driver.create_domain(domain_id, domain)"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa98f980_a29edf04","line":313,"updated":"2014-09-05 16:35:37.000000000","message":"a custom message here would provide a better user feedback than a generic 403 message.\n\ni\u0027m also not sure that a 403 is the most appropriate status. it\u0027s almost a 400 error instead. a 403 implies the request can be repeated with different auth and succeed, which isn\u0027t the case here. a 400 suggests that a different value in the request might cause it to succeed (like a different domain ID, which is true here).","commit_id":"7db852705831ef739ad18092aeb3a8545a6ab210"},{"author":{"_account_id":12215,"name":"Alexey Miroshkin","email":"alexey.miroshkin@ru.ibm.com","username":"amirosh"},"change_message_id":"935c5b9d168bb69e518d4f8fa02e69774ebc5f4c","unresolved":false,"context_lines":[{"line_number":310,"context_line":"    def create_domain(self, domain_id, domain):"},{"line_number":311,"context_line":"        if (not self.identity_api.multiple_domains_support() and"},{"line_number":312,"context_line":"                domain_id !\u003d CONF.identity.default_domain_id):"},{"line_number":313,"context_line":"            raise exception.Forbidden()"},{"line_number":314,"context_line":"        domain.setdefault(\u0027enabled\u0027, True)"},{"line_number":315,"context_line":"        domain[\u0027enabled\u0027] \u003d clean.domain_enabled(domain[\u0027enabled\u0027])"},{"line_number":316,"context_line":"        ret \u003d self.driver.create_domain(domain_id, domain)"}],"source_content_type":"text/x-python","patch_set":2,"id":"fa98f980_99f5cdee","line":313,"in_reply_to":"fa98f980_a29edf04","updated":"2014-09-05 20:27:47.000000000","message":"Agree, but 403 was used to preserve compatibility with LDAP driver which returns it in domain crud operations:\n\nraise exception.Forbidden(_(\u0027Domains are read-only against LDAP\u0027))\n\nso at least test_domain_crud in test_backedn_ldap will fail. Should we update LDAP driver to throw 400 instead of 403? It would be a separate change, I suppose.","commit_id":"7db852705831ef739ad18092aeb3a8545a6ab210"}],"keystone/identity/core.py":[{"author":{"_account_id":4,"name":"Dolph Mathews","email":"dolph.mathews@gmail.com","username":"dolph"},"change_message_id":"42f78a0086e7f90856057b4326b4cbc309f8076b","unresolved":false,"context_lines":[{"line_number":843,"context_line":"        \"\"\"Indicates if Driver supports domains.\"\"\""},{"line_number":844,"context_line":"        return True"},{"line_number":845,"context_line":""},{"line_number":846,"context_line":"    def multiple_domains_support(self):"},{"line_number":847,"context_line":"        return (self.is_domain_aware() or"},{"line_number":848,"context_line":"                CONF.identity.domain_specific_drivers_enabled)"},{"line_number":849,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"fa98f980_42a4534a","line":846,"updated":"2014-09-05 16:35:37.000000000","message":"(grammar) i\u0027m not sure the plurality of \"domains\" makes sense here, and given that there\u0027s a cost to change method names in these interfaces later, i\u0027m going to suggest any one of:\n\n  multiple_domain_support()\n\n  multiple_domains_supported()\n\n(i\u0027m not really sure which you intended, but they all work for me)\n\nthis should also probably be a @property (in which case _supported might make the most sense in usage)","commit_id":"7db852705831ef739ad18092aeb3a8545a6ab210"},{"author":{"_account_id":12215,"name":"Alexey Miroshkin","email":"alexey.miroshkin@ru.ibm.com","username":"amirosh"},"change_message_id":"935c5b9d168bb69e518d4f8fa02e69774ebc5f4c","unresolved":false,"context_lines":[{"line_number":843,"context_line":"        \"\"\"Indicates if Driver supports domains.\"\"\""},{"line_number":844,"context_line":"        return True"},{"line_number":845,"context_line":""},{"line_number":846,"context_line":"    def multiple_domains_support(self):"},{"line_number":847,"context_line":"        return (self.is_domain_aware() or"},{"line_number":848,"context_line":"                CONF.identity.domain_specific_drivers_enabled)"},{"line_number":849,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"fa98f980_730be87c","line":846,"in_reply_to":"fa98f980_42a4534a","updated":"2014-09-05 20:27:47.000000000","message":"I like your suggestion about @property, agree that multiple_domains_supported makes more sense. Will fix it in the next patch.","commit_id":"7db852705831ef739ad18092aeb3a8545a6ab210"}]}