)]}'
{"swift/proxy/controllers/base.py":[{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"b8e90fa9f4b05dd86833d0d59e18ea5fb362df02","unresolved":false,"context_lines":[{"line_number":238,"context_line":"                expose_headers \u003d set(["},{"line_number":239,"context_line":"                    \u0027cache-control\u0027, \u0027content-language\u0027, \u0027content-type\u0027,"},{"line_number":240,"context_line":"                    \u0027expires\u0027, \u0027last-modified\u0027, \u0027pragma\u0027, \u0027etag\u0027,"},{"line_number":241,"context_line":"                    \u0027x-timestamp\u0027, \u0027x-trans-id\u0027])"},{"line_number":242,"context_line":"                for header in resp.headers:"},{"line_number":243,"context_line":"                    if header.startswith(\u0027X-Container-Meta\u0027) or \\"},{"line_number":244,"context_line":"                            header.startswith(\u0027X-Object-Meta\u0027):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a7e1126_1edd0ee0","line":241,"updated":"2015-12-21 01:52:02.000000000","message":"Making a list is not needed. Just use set() directly.\n\nLike:\n expose_headers \u003d set(\n     * snip *\n     )\n\nThis will reduce memory usage rather than list after set.","commit_id":"647f3f6be25df03db83ef3d7e89831a4ec29491c"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"ba87eee2403f0d8cef76462df385daaee6e7e142","unresolved":false,"context_lines":[{"line_number":238,"context_line":"                expose_headers \u003d set(["},{"line_number":239,"context_line":"                    \u0027cache-control\u0027, \u0027content-language\u0027, \u0027content-type\u0027,"},{"line_number":240,"context_line":"                    \u0027expires\u0027, \u0027last-modified\u0027, \u0027pragma\u0027, \u0027etag\u0027,"},{"line_number":241,"context_line":"                    \u0027x-timestamp\u0027, \u0027x-trans-id\u0027])"},{"line_number":242,"context_line":"                for header in resp.headers:"},{"line_number":243,"context_line":"                    if header.startswith(\u0027X-Container-Meta\u0027) or \\"},{"line_number":244,"context_line":"                            header.startswith(\u0027X-Object-Meta\u0027):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a7e1126_874c0ac9","line":241,"in_reply_to":"3a7e1126_1edd0ee0","updated":"2015-12-21 07:27:10.000000000","message":"I think you have misunderstanding parameter for set().\nIt is required \"one\" iteratable object. Please check the following :-)\n\nhttps://docs.python.org/2.7/library/stdtypes.html?highlight\u003dset#set-types-set-frozenset","commit_id":"647f3f6be25df03db83ef3d7e89831a4ec29491c"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"79d4496c929308e0c49a33acc4968e7327dafc1a","unresolved":false,"context_lines":[{"line_number":238,"context_line":"                expose_headers \u003d set(["},{"line_number":239,"context_line":"                    \u0027cache-control\u0027, \u0027content-language\u0027, \u0027content-type\u0027,"},{"line_number":240,"context_line":"                    \u0027expires\u0027, \u0027last-modified\u0027, \u0027pragma\u0027, \u0027etag\u0027,"},{"line_number":241,"context_line":"                    \u0027x-timestamp\u0027, \u0027x-trans-id\u0027])"},{"line_number":242,"context_line":"                for header in resp.headers:"},{"line_number":243,"context_line":"                    if header.startswith(\u0027X-Container-Meta\u0027) or \\"},{"line_number":244,"context_line":"                            header.startswith(\u0027X-Object-Meta\u0027):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a7e1126_2dbe5465","line":241,"in_reply_to":"3a7e1126_874c0ac9","updated":"2015-12-21 08:00:51.000000000","message":"Oh, yeah. My bad. Sorry.\n\nJust i thought \"a \u003d set() -\u003e a.add(item)\" vs \"a \u003d list() -\u003e a.append(item)\" but this is not.","commit_id":"647f3f6be25df03db83ef3d7e89831a4ec29491c"},{"author":{"_account_id":4608,"name":"Kota Tsuyuzaki","email":"bloodeagle40234@gmail.com","username":"tsuyuzaki-kota"},"change_message_id":"b8e90fa9f4b05dd86833d0d59e18ea5fb362df02","unresolved":false,"context_lines":[{"line_number":248,"context_line":"                        [header_line.strip().lower()"},{"line_number":249,"context_line":"                         for header_line in"},{"line_number":250,"context_line":"                         cors_info[\u0027expose_headers\u0027].split(\u0027 \u0027)"},{"line_number":251,"context_line":"                         if header_line.strip()])"},{"line_number":252,"context_line":"                resp.headers[\u0027Access-Control-Expose-Headers\u0027] \u003d \\"},{"line_number":253,"context_line":"                    \u0027, \u0027.join(expose_headers)"},{"line_number":254,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3a7e1126_a172560f","line":251,"updated":"2015-12-21 01:52:02.000000000","message":"It might be only me to think so but I like use Title case rather than lower case for the response headers.\n\nLooking at docs[1], X-Object-Meta-xxx seems in title case but some headers like x-trans-id are in lower case. Not sure which is suitable, how do you think of?\n\n1: http://docs.openstack.org/developer/swift/cors.html","commit_id":"647f3f6be25df03db83ef3d7e89831a4ec29491c"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"ba87eee2403f0d8cef76462df385daaee6e7e142","unresolved":false,"context_lines":[{"line_number":248,"context_line":"                        [header_line.strip().lower()"},{"line_number":249,"context_line":"                         for header_line in"},{"line_number":250,"context_line":"                         cors_info[\u0027expose_headers\u0027].split(\u0027 \u0027)"},{"line_number":251,"context_line":"                         if header_line.strip()])"},{"line_number":252,"context_line":"                resp.headers[\u0027Access-Control-Expose-Headers\u0027] \u003d \\"},{"line_number":253,"context_line":"                    \u0027, \u0027.join(expose_headers)"},{"line_number":254,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3a7e1126_47338214","line":251,"in_reply_to":"3a7e1126_a172560f","updated":"2015-12-21 07:27:10.000000000","message":"HEADERS are NOT case-sensitive in RFC so I think both OK but current implementation uses lower case for value of  Access-Control-Expose-Headers.","commit_id":"647f3f6be25df03db83ef3d7e89831a4ec29491c"}],"test/unit/proxy/controllers/test_base.py":[{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"f857e8750798256359c698983aa9c44462da134a","unresolved":false,"context_lines":[{"line_number":646,"context_line":"        self.app.cors_allow_origin \u003d []"},{"line_number":647,"context_line":"        self.app.strict_cors_mode \u003d False"},{"line_number":648,"context_line":"        base \u003d Controller(self.app)"},{"line_number":649,"context_line":"        base.container_info \u003d my_container_info"},{"line_number":650,"context_line":"        base.account_name \u003d \u0027a\u0027"},{"line_number":651,"context_line":"        base.container_name \u003d \u0027c\u0027"},{"line_number":652,"context_line":"        req \u003d Request.blank(\u0027/v1/a/c/o\u0027,"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a7e1126_0fdf226a","line":649,"updated":"2015-12-18 01:33:22.000000000","message":"Instead of monkey patching yourself, which isn\u0027t necessarily cleaned up afterwards and that the fact that mock\u0027s patch method has already been imported, it would be better to use that:\n\n  with patch(\u0027swift.proxy.controllers.base.Controller.container_info\u0027,                                                                                                                                                                                        \n              my_container_info):                                                                                                                                                                                                                             \n      base \u003d Controller(self.app)                                                                                                                                                                                                                             \n      base.account_name \u003d \u0027a\u0027                                                                                                                                                                                                                                 \n      base.container_name \u003d \u0027c\u0027                                                                                                                                                                                                                               \n      req \u003d Request.blank(\u0027/v1/a/c/o\u0027,                                                                                                                                                                                                                        \n                          environ\u003d{\u0027swift.cache\u0027: FakeCache()},                                                                                                                                                                                               \n                          headers\u003d{\u0027Origin\u0027: \u0027http://m.com\u0027})                                                                                                                                                                                                 \n                                                                                                                                                                                                                                                              \n      resp \u003d GET(base, req)                                                                                                                                                                                                                                   \n  self.assertEqual(resp.status_int, 200)\n\nObviously here, and in the tests below.","commit_id":"791e60919109608504c4bbdf3c436dd0b4010f1a"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"852b6101faf149b3977ea908af11329d4d2d1474","unresolved":false,"context_lines":[{"line_number":625,"context_line":"        self.assertEqual(base.have_quorum([404, 404], 2), True)"},{"line_number":626,"context_line":"        self.assertEqual(base.have_quorum([201, 404, 201, 201], 4), True)"},{"line_number":627,"context_line":""},{"line_number":628,"context_line":"    def test_cors_validation(self):"},{"line_number":629,"context_line":""},{"line_number":630,"context_line":"        @cors_validation"},{"line_number":631,"context_line":"        def GET(controller, req):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a7e1126_9776a3a2","line":628,"updated":"2015-12-18 15:38:18.000000000","message":"nit: perhaps name the method with more information e.g. test_cors_validation_expose_headers","commit_id":"647f3f6be25df03db83ef3d7e89831a4ec29491c"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"ba87eee2403f0d8cef76462df385daaee6e7e142","unresolved":false,"context_lines":[{"line_number":625,"context_line":"        self.assertEqual(base.have_quorum([404, 404], 2), True)"},{"line_number":626,"context_line":"        self.assertEqual(base.have_quorum([201, 404, 201, 201], 4), True)"},{"line_number":627,"context_line":""},{"line_number":628,"context_line":"    def test_cors_validation(self):"},{"line_number":629,"context_line":""},{"line_number":630,"context_line":"        @cors_validation"},{"line_number":631,"context_line":"        def GET(controller, req):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a7e1126_4f0b988e","line":628,"in_reply_to":"3a7e1126_9776a3a2","updated":"2015-12-21 07:27:10.000000000","message":"I will merge tests to test_server.py so this fix is not necessary anymore. thanks for advice, when I will submit other patches, I will check this point.","commit_id":"647f3f6be25df03db83ef3d7e89831a4ec29491c"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"852b6101faf149b3977ea908af11329d4d2d1474","unresolved":false,"context_lines":[{"line_number":667,"context_line":"        self.assertIn(\u0027x-object-meta-my-object-header2\u0027,"},{"line_number":668,"context_line":"                      resp.headers[\u0027Access-Control-Expose-Headers\u0027])"},{"line_number":669,"context_line":"        self.assertIn(\u0027*\u0027,"},{"line_number":670,"context_line":"                      resp.headers[\u0027Access-Control-Allow-Origin\u0027])"},{"line_number":671,"context_line":""},{"line_number":672,"context_line":"    def test_cors_validation_no_allow_origin(self):"},{"line_number":673,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3a7e1126_b7063f1b","line":670,"updated":"2015-12-18 15:38:18.000000000","message":"OK. This test verifies that when X-Object-Meta-My-Object-Header is in response headers and in expose_headers then it only appears once in Access-Control-Expose-Headers list.","commit_id":"647f3f6be25df03db83ef3d7e89831a4ec29491c"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"ba87eee2403f0d8cef76462df385daaee6e7e142","unresolved":false,"context_lines":[{"line_number":667,"context_line":"        self.assertIn(\u0027x-object-meta-my-object-header2\u0027,"},{"line_number":668,"context_line":"                      resp.headers[\u0027Access-Control-Expose-Headers\u0027])"},{"line_number":669,"context_line":"        self.assertIn(\u0027*\u0027,"},{"line_number":670,"context_line":"                      resp.headers[\u0027Access-Control-Allow-Origin\u0027])"},{"line_number":671,"context_line":""},{"line_number":672,"context_line":"    def test_cors_validation_no_allow_origin(self):"},{"line_number":673,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3a7e1126_4fce382f","line":670,"in_reply_to":"3a7e1126_b7063f1b","updated":"2015-12-21 07:27:10.000000000","message":"This test (checking \u0027*\u0027 in resp.headers[\u0027access_control-allow-origin\u0027]) is NOT in test_server.py so I will add this in test_server.py.","commit_id":"647f3f6be25df03db83ef3d7e89831a4ec29491c"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"852b6101faf149b3977ea908af11329d4d2d1474","unresolved":false,"context_lines":[{"line_number":669,"context_line":"        self.assertIn(\u0027*\u0027,"},{"line_number":670,"context_line":"                      resp.headers[\u0027Access-Control-Allow-Origin\u0027])"},{"line_number":671,"context_line":""},{"line_number":672,"context_line":"    def test_cors_validation_no_allow_origin(self):"},{"line_number":673,"context_line":""},{"line_number":674,"context_line":"        @cors_validation"},{"line_number":675,"context_line":"        def GET(controller, req):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a7e1126_17987377","line":672,"updated":"2015-12-18 15:38:18.000000000","message":"this test is exactly the same as the one above but with allow_origin \u003d \u0027\u0027\n\nComparing to the tests in test/unit/proxy/test_server.py, I don\u0027t think this variation of allow_origin is tested there, so this is a good addition. But in test_server.py the tests verify all the expected expose_headers, which this test doesn\u0027t e.g. test/unit/proxy/test_server.py:5879","commit_id":"647f3f6be25df03db83ef3d7e89831a4ec29491c"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"ba87eee2403f0d8cef76462df385daaee6e7e142","unresolved":false,"context_lines":[{"line_number":669,"context_line":"        self.assertIn(\u0027*\u0027,"},{"line_number":670,"context_line":"                      resp.headers[\u0027Access-Control-Allow-Origin\u0027])"},{"line_number":671,"context_line":""},{"line_number":672,"context_line":"    def test_cors_validation_no_allow_origin(self):"},{"line_number":673,"context_line":""},{"line_number":674,"context_line":"        @cors_validation"},{"line_number":675,"context_line":"        def GET(controller, req):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a7e1126_4fc398ed","line":672,"in_reply_to":"3a7e1126_17987377","updated":"2015-12-21 07:27:10.000000000","message":"I will merge the test in test_server.py.","commit_id":"647f3f6be25df03db83ef3d7e89831a4ec29491c"},{"author":{"_account_id":7847,"name":"Alistair Coles","email":"alistairncoles@gmail.com","username":"acoles"},"change_message_id":"852b6101faf149b3977ea908af11329d4d2d1474","unresolved":false,"context_lines":[{"line_number":713,"context_line":"        self.assertIn(\u0027http://m.com\u0027,"},{"line_number":714,"context_line":"                      resp.headers[\u0027Access-Control-Allow-Origin\u0027])"},{"line_number":715,"context_line":""},{"line_number":716,"context_line":"    def test_cors_validation_not_allowed(self):"},{"line_number":717,"context_line":"        @cors_validation"},{"line_number":718,"context_line":"        def GET(controller, req):"},{"line_number":719,"context_line":"            headers \u003d {"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a7e1126_b7195fd9","line":716,"updated":"2015-12-18 15:38:18.000000000","message":"this test passes on master, and seems to duplicate the tests here: test/unit/proxy/test_server.py:5879 except here you also check that Access-Control-Expose-Headers is not in the response headers. Could we just add that assertion to the existing test in test_server.py?","commit_id":"647f3f6be25df03db83ef3d7e89831a4ec29491c"},{"author":{"_account_id":12193,"name":"Hisashi Osanai","email":"osanai.hisashi@jp.fujitsu.com","username":"ho"},"change_message_id":"ba87eee2403f0d8cef76462df385daaee6e7e142","unresolved":false,"context_lines":[{"line_number":713,"context_line":"        self.assertIn(\u0027http://m.com\u0027,"},{"line_number":714,"context_line":"                      resp.headers[\u0027Access-Control-Allow-Origin\u0027])"},{"line_number":715,"context_line":""},{"line_number":716,"context_line":"    def test_cors_validation_not_allowed(self):"},{"line_number":717,"context_line":"        @cors_validation"},{"line_number":718,"context_line":"        def GET(controller, req):"},{"line_number":719,"context_line":"            headers \u003d {"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a7e1126_dc4ae425","line":716,"in_reply_to":"3a7e1126_b7195fd9","updated":"2015-12-21 07:27:10.000000000","message":"Done","commit_id":"647f3f6be25df03db83ef3d7e89831a4ec29491c"}]}