)]}'
{"keystone/policy/controllers.py":[{"author":{"_account_id":7725,"name":"David Stanek","email":"dstanek@dstanek.com","username":"dstanek"},"change_message_id":"26d5149d0338b228a6e9d84fdbf2a2aee2777d62","unresolved":false,"context_lines":[{"line_number":45,"context_line":"    @controller.protected()"},{"line_number":46,"context_line":"    @validation.validated(schema.policy_update, \u0027policy\u0027)"},{"line_number":47,"context_line":"    def update_policy(self, context, policy_id, policy):"},{"line_number":48,"context_line":"        self._require_matching_id(policy_id, policy)"},{"line_number":49,"context_line":"        ref \u003d self.policy_api.update_policy(policy_id, policy)"},{"line_number":50,"context_line":"        return PolicyV3.wrap_member(context, ref)"},{"line_number":51,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5a890539_36e315a5","line":48,"updated":"2014-11-26 04:55:51.000000000","message":"I\u0027m not sure about this. I know you are saying that this makes it the same as other places, but I would push back and say that those other places may be wrong. To me this is business logic for the manager layer.","commit_id":"1183b75dec35f3d15cf51bbc6e3555030fad52ff"},{"author":{"_account_id":9101,"name":"wanghong","email":"reviewwanghong@126.com","username":"wanghong"},"change_message_id":"e669dbeeb4dbf03a98b58f0e7e01abd4a162f8a0","unresolved":false,"context_lines":[{"line_number":45,"context_line":"    @controller.protected()"},{"line_number":46,"context_line":"    @validation.validated(schema.policy_update, \u0027policy\u0027)"},{"line_number":47,"context_line":"    def update_policy(self, context, policy_id, policy):"},{"line_number":48,"context_line":"        self._require_matching_id(policy_id, policy)"},{"line_number":49,"context_line":"        ref \u003d self.policy_api.update_policy(policy_id, policy)"},{"line_number":50,"context_line":"        return PolicyV3.wrap_member(context, ref)"},{"line_number":51,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"5a890539_4789c96d","line":48,"in_reply_to":"5a890539_36e315a5","updated":"2014-11-26 07:34:14.000000000","message":"I don\u0027t think so. I think it is right that check input parameters in controller layer.","commit_id":"1183b75dec35f3d15cf51bbc6e3555030fad52ff"},{"author":{"_account_id":13055,"name":"Alexander Makarov","email":"ajiekcahdp.makapob@gmail.com","username":"amakarov"},"change_message_id":"66c4ff35a0016d93f7c27a75b2993adb62809048","unresolved":false,"context_lines":[{"line_number":45,"context_line":"    @controller.protected()"},{"line_number":46,"context_line":"    @validation.validated(schema.policy_update, \u0027policy\u0027)"},{"line_number":47,"context_line":"    def update_policy(self, context, policy_id, policy):"},{"line_number":48,"context_line":"        self._require_matching_id(policy_id, policy)"},{"line_number":49,"context_line":"        ref \u003d self.policy_api.update_policy(policy_id, policy)"},{"line_number":50,"context_line":"        return PolicyV3.wrap_member(context, ref)"},{"line_number":51,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"5a890539_6c05fcc0","line":48,"updated":"2014-12-03 17:28:48.000000000","message":"It\u0027s sort of a trend in the community to move validations relevant to operation to it\u0027s manager.\nI think it\u0027ll be better if done in the manager as this validation will make manager itself more consistent.","commit_id":"689eb8b3c520f2d66f78375e34a5bc05fc4b17d8"},{"author":{"_account_id":7725,"name":"David Stanek","email":"dstanek@dstanek.com","username":"dstanek"},"change_message_id":"2640d3fee3b699694c0e4b1af47eed05121a9fcc","unresolved":false,"context_lines":[{"line_number":45,"context_line":"    @controller.protected()"},{"line_number":46,"context_line":"    @validation.validated(schema.policy_update, \u0027policy\u0027)"},{"line_number":47,"context_line":"    def update_policy(self, context, policy_id, policy):"},{"line_number":48,"context_line":"        self._require_matching_id(policy_id, policy)"},{"line_number":49,"context_line":"        ref \u003d self.policy_api.update_policy(policy_id, policy)"},{"line_number":50,"context_line":"        return PolicyV3.wrap_member(context, ref)"},{"line_number":51,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"5a890539_3b64deb1","line":48,"in_reply_to":"5a890539_6c05fcc0","updated":"2014-12-04 15:25:09.000000000","message":"++\n\nThe controller layer is the web layer. It should be responsible for checking input parameters against a schema and other web-based responsibilities. The manager layer is the business logic that should know what the parameters mean to the system.","commit_id":"689eb8b3c520f2d66f78375e34a5bc05fc4b17d8"}],"keystone/policy/core.py":[{"author":{"_account_id":7725,"name":"David Stanek","email":"dstanek@dstanek.com","username":"dstanek"},"change_message_id":"26d5149d0338b228a6e9d84fdbf2a2aee2777d62","unresolved":false,"context_lines":[{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    @notifications.updated(_POLICY, public\u003dFalse)"},{"line_number":55,"context_line":"    def update_policy(self, policy_id, policy):"},{"line_number":56,"context_line":"        if \u0027id\u0027 in policy and policy_id !\u003d policy[\u0027id\u0027]:"},{"line_number":57,"context_line":"            raise exception.ValidationError(\u0027Cannot change policy ID\u0027)"},{"line_number":58,"context_line":"        try:"},{"line_number":59,"context_line":"            return self.driver.update_policy(policy_id, policy)"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a890539_5647c1a2","side":"PARENT","line":56,"updated":"2014-11-26 04:55:51.000000000","message":"If something calls this method with invalid data do we no longer care?","commit_id":"3d9184b6f5860f0b56091a326ed41d2a4c29fbe4"},{"author":{"_account_id":9101,"name":"wanghong","email":"reviewwanghong@126.com","username":"wanghong"},"change_message_id":"e669dbeeb4dbf03a98b58f0e7e01abd4a162f8a0","unresolved":false,"context_lines":[{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    @notifications.updated(_POLICY, public\u003dFalse)"},{"line_number":55,"context_line":"    def update_policy(self, policy_id, policy):"},{"line_number":56,"context_line":"        if \u0027id\u0027 in policy and policy_id !\u003d policy[\u0027id\u0027]:"},{"line_number":57,"context_line":"            raise exception.ValidationError(\u0027Cannot change policy ID\u0027)"},{"line_number":58,"context_line":"        try:"},{"line_number":59,"context_line":"            return self.driver.update_policy(policy_id, policy)"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a890539_a7c8ada8","side":"PARENT","line":56,"in_reply_to":"5a890539_5647c1a2","updated":"2014-11-26 07:34:14.000000000","message":"I think this method should only be called by controller layer.","commit_id":"3d9184b6f5860f0b56091a326ed41d2a4c29fbe4"}],"keystone/tests/test_backend.py":[{"author":{"_account_id":13063,"name":"WEI CHEN","email":"dave.jungler@gmail.com","username":"wei.d.chen"},"change_message_id":"ad96ef4b1ec70f1872bcda25410734d88534330d","unresolved":false,"context_lines":[{"line_number":4187,"context_line":""},{"line_number":4188,"context_line":"        policy[\u0027id\u0027] \u003d \u0027fake2\u0027"},{"line_number":4189,"context_line":"        self.policy_api.update_policy(\u0027fake1\u0027, policy)"},{"line_number":4190,"context_line":""},{"line_number":4191,"context_line":"        policy_ref \u003d self.policy_api.get_policy(\u0027fake1\u0027)"},{"line_number":4192,"context_line":"        self.assertEqual(\u0027fake1\u0027, policy_ref[\u0027id\u0027])"},{"line_number":4193,"context_line":"        self.assertRaises(exception.PolicyNotFound,"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a890539_2af69863","line":4190,"updated":"2014-11-14 11:31:39.000000000","message":"Are you sure there is no exception thrown here? What i saw is the exception is still thrown within the method of \"_require_matching_id\"","commit_id":"1183b75dec35f3d15cf51bbc6e3555030fad52ff"},{"author":{"_account_id":9101,"name":"wanghong","email":"reviewwanghong@126.com","username":"wanghong"},"change_message_id":"aa5ffc83f70c82d29b21035bce5473e38754bda1","unresolved":false,"context_lines":[{"line_number":4187,"context_line":""},{"line_number":4188,"context_line":"        policy[\u0027id\u0027] \u003d \u0027fake2\u0027"},{"line_number":4189,"context_line":"        self.policy_api.update_policy(\u0027fake1\u0027, policy)"},{"line_number":4190,"context_line":""},{"line_number":4191,"context_line":"        policy_ref \u003d self.policy_api.get_policy(\u0027fake1\u0027)"},{"line_number":4192,"context_line":"        self.assertEqual(\u0027fake1\u0027, policy_ref[\u0027id\u0027])"},{"line_number":4193,"context_line":"        self.assertRaises(exception.PolicyNotFound,"}],"source_content_type":"text/x-python","patch_set":2,"id":"5a890539_35942642","line":4190,"in_reply_to":"5a890539_2af69863","updated":"2014-11-15 01:02:15.000000000","message":"Note that the tests in test_xx_backend.py or test_backend.py directly call the apis in manager level(in here is self.policy_api). That is say, we don\u0027t pass the calls into controller level. Actually, the test in test_v3_policy.py tests that it will raise exception within _require_matching_id method.","commit_id":"1183b75dec35f3d15cf51bbc6e3555030fad52ff"}]}