)]}'
{"cinder/api/v2/views/volumes.py":[{"author":{"_account_id":1736,"name":"Ivan Kolodyazhny","email":"e0ne@e0ne.info","username":"e0ne"},"change_message_id":"1f9dec3fbbcd6658f7e1105a50ff7ca169089dc6","unresolved":false,"context_lines":[{"line_number":94,"context_line":"        ctxt \u003d request.environ[\u0027cinder.context\u0027]"},{"line_number":95,"context_line":"        if not ctxt.is_admin:"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"            if volume_ref[\u0027volume\u0027][\u0027attachments\u0027]:"},{"line_number":98,"context_line":"                for att in range(len(volume_ref[\u0027volume\u0027][\u0027attachments\u0027])):"},{"line_number":99,"context_line":"                    volume_ref[\u0027volume\u0027][\u0027attachments\u0027][att]["},{"line_number":100,"context_line":"                        \u0027host_name\u0027] \u003d None"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff570b3c_f0f9014e","line":97,"range":{"start_line":97,"start_character":15,"end_line":97,"end_character":50},"updated":"2020-06-02 13:14:17.000000000","message":"Do we want to leave existing behaviour here and leave:\n if volume_ref.get(\u0027attachments\u0027):\nit looks safer","commit_id":"fb48c993bf03880d8c88bbb03b5fea39df9c6036"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"60f3acc913f176462a3a7d999b9f37edf47037d9","unresolved":false,"context_lines":[{"line_number":94,"context_line":"        ctxt \u003d request.environ[\u0027cinder.context\u0027]"},{"line_number":95,"context_line":"        if not ctxt.is_admin:"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"            if volume_ref[\u0027volume\u0027][\u0027attachments\u0027]:"},{"line_number":98,"context_line":"                for att in range(len(volume_ref[\u0027volume\u0027][\u0027attachments\u0027])):"},{"line_number":99,"context_line":"                    volume_ref[\u0027volume\u0027][\u0027attachments\u0027][att]["},{"line_number":100,"context_line":"                        \u0027host_name\u0027] \u003d None"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff570b3c_46ebd376","line":97,"range":{"start_line":97,"start_character":15,"end_line":97,"end_character":50},"in_reply_to":"ff570b3c_63a7e9bd","updated":"2020-06-02 14:45:22.000000000","message":"If we use .get then it will check if the key exists and not it\u0027s value (which can be empty)\nwe can use .get and check the value after that\n@Sean, yes, that was wrong and also handling case for multiple attachments which wasn\u0027t handled before.","commit_id":"fb48c993bf03880d8c88bbb03b5fea39df9c6036"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"9f14add425abc7f798780f9eeb721f4619c912f5","unresolved":false,"context_lines":[{"line_number":94,"context_line":"        ctxt \u003d request.environ[\u0027cinder.context\u0027]"},{"line_number":95,"context_line":"        if not ctxt.is_admin:"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"            if volume_ref[\u0027volume\u0027][\u0027attachments\u0027]:"},{"line_number":98,"context_line":"                for att in range(len(volume_ref[\u0027volume\u0027][\u0027attachments\u0027])):"},{"line_number":99,"context_line":"                    volume_ref[\u0027volume\u0027][\u0027attachments\u0027][att]["},{"line_number":100,"context_line":"                        \u0027host_name\u0027] \u003d None"}],"source_content_type":"text/x-python","patch_set":2,"id":"ff570b3c_63a7e9bd","line":97,"range":{"start_line":97,"start_character":15,"end_line":97,"end_character":50},"in_reply_to":"ff570b3c_f0f9014e","updated":"2020-06-02 13:40:06.000000000","message":"I agree. Commit message says attachments will always be there, but what about the case when upgrading from a release before the new attachment changes. I think in that case, there may be a chance this is not present. So just to be safe, I would rather see a get() call here.\n\nI\u0027m a little confused by the difference here though. The old code checked volume_ref[\u0027attachments\u0027] but this checks volume_ref[\u0027volume\u0027][\u0027attachments\u0027]. Was the old code wrong in the if check (it did use the deeper structure in the body of the condition block.","commit_id":"fb48c993bf03880d8c88bbb03b5fea39df9c6036"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"a0bd9049a6fc94858a17033986b7f861f72c5ebf","unresolved":false,"context_lines":[{"line_number":95,"context_line":"        if not ctxt.is_admin:"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"            if (volume_ref[\u0027volume\u0027].get(\u0027attachments\u0027) and"},{"line_number":98,"context_line":"                    volume_ref[\u0027volume\u0027][\u0027attachments\u0027]):"},{"line_number":99,"context_line":"                for att in range(len(volume_ref[\u0027volume\u0027][\u0027attachments\u0027])):"},{"line_number":100,"context_line":"                    volume_ref[\u0027volume\u0027][\u0027attachments\u0027][att]["},{"line_number":101,"context_line":"                        \u0027host_name\u0027] \u003d None"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff570b3c_41845d5e","line":98,"range":{"start_line":98,"start_character":20,"end_line":98,"end_character":55},"updated":"2020-06-02 15:00:33.000000000","message":"This is redundant.","commit_id":"744a7cfd7d5775003e584be994024f9e7f86e298"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"d3869eb6296de0cbb3b0265c966305a5fe53b6c3","unresolved":false,"context_lines":[{"line_number":95,"context_line":"        if not ctxt.is_admin:"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"            if (volume_ref[\u0027volume\u0027].get(\u0027attachments\u0027) and"},{"line_number":98,"context_line":"                    volume_ref[\u0027volume\u0027][\u0027attachments\u0027]):"},{"line_number":99,"context_line":"                for att in range(len(volume_ref[\u0027volume\u0027][\u0027attachments\u0027])):"},{"line_number":100,"context_line":"                    volume_ref[\u0027volume\u0027][\u0027attachments\u0027][att]["},{"line_number":101,"context_line":"                        \u0027host_name\u0027] \u003d None"}],"source_content_type":"text/x-python","patch_set":3,"id":"ff570b3c_a147391d","line":98,"range":{"start_line":98,"start_character":20,"end_line":98,"end_character":55},"in_reply_to":"ff570b3c_41845d5e","updated":"2020-06-02 15:07:24.000000000","message":"right. don\u0027t know what i was thinking. thanks","commit_id":"744a7cfd7d5775003e584be994024f9e7f86e298"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"42989524f5383cd419f1de6b4ca47ea8aa31ad6a","unresolved":false,"context_lines":[{"line_number":92,"context_line":"            }"},{"line_number":93,"context_line":"        }"},{"line_number":94,"context_line":"        ctxt \u003d request.environ[\u0027cinder.context\u0027]"},{"line_number":95,"context_line":"        if not ctxt.is_admin:"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"            if volume_ref[\u0027volume\u0027].get(\u0027attachments\u0027):"},{"line_number":98,"context_line":"                for att in range(len(volume_ref[\u0027volume\u0027][\u0027attachments\u0027])):"},{"line_number":99,"context_line":"                    volume_ref[\u0027volume\u0027][\u0027attachments\u0027][att]["},{"line_number":100,"context_line":"                        \u0027host_name\u0027] \u003d None"},{"line_number":101,"context_line":"        if ctxt.is_admin:"},{"line_number":102,"context_line":"            volume_ref[\u0027volume\u0027][\u0027migration_status\u0027] \u003d ("},{"line_number":103,"context_line":"                volume.get(\u0027migration_status\u0027))"}],"source_content_type":"text/x-python","patch_set":4,"id":"ff570b3c_ae715a0f","line":100,"range":{"start_line":95,"start_character":0,"end_line":100,"end_character":43},"updated":"2020-06-10 14:56:47.000000000","message":"-1: I think we should do this in the _get_attachment method by passing the request variable, or just passing ctx.is_admin","commit_id":"cb99646dfe18c3652b4123a3fb9c6b913db8e74b"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"fa9ba043f822d281f97dcc30fc3c862a7ca87d47","unresolved":false,"context_lines":[{"line_number":92,"context_line":"            }"},{"line_number":93,"context_line":"        }"},{"line_number":94,"context_line":"        ctxt \u003d request.environ[\u0027cinder.context\u0027]"},{"line_number":95,"context_line":"        if not ctxt.is_admin:"},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"            if volume_ref[\u0027volume\u0027].get(\u0027attachments\u0027):"},{"line_number":98,"context_line":"                for att in range(len(volume_ref[\u0027volume\u0027][\u0027attachments\u0027])):"},{"line_number":99,"context_line":"                    volume_ref[\u0027volume\u0027][\u0027attachments\u0027][att]["},{"line_number":100,"context_line":"                        \u0027host_name\u0027] \u003d None"},{"line_number":101,"context_line":"        if ctxt.is_admin:"},{"line_number":102,"context_line":"            volume_ref[\u0027volume\u0027][\u0027migration_status\u0027] \u003d ("},{"line_number":103,"context_line":"                volume.get(\u0027migration_status\u0027))"}],"source_content_type":"text/x-python","patch_set":4,"id":"ff570b3c_75f70d8e","line":100,"range":{"start_line":95,"start_character":0,"end_line":100,"end_character":43},"in_reply_to":"ff570b3c_ae715a0f","updated":"2020-06-10 18:33:57.000000000","message":"Done","commit_id":"cb99646dfe18c3652b4123a3fb9c6b913db8e74b"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"c942befbbbb43014939cfada8d47fd8c0c853037","unresolved":false,"context_lines":[{"line_number":115,"context_line":"        \"\"\"Determine if volume is encrypted.\"\"\""},{"line_number":116,"context_line":"        return volume.get(\u0027encryption_key_id\u0027) is not None"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    def _get_attachments(self, is_admin, volume):"},{"line_number":119,"context_line":"        \"\"\"Retrieve the attachments of the volume object.\"\"\""},{"line_number":120,"context_line":"        attachments \u003d []"},{"line_number":121,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"bf51134e_e4539f6b","line":118,"updated":"2020-06-30 19:04:20.000000000","message":"Nit: i\u0027d suggest making the is_admin the second parameter with a default value of False, and that way you won\u0027t break any current calls to _get_attachments() that only pass the volume (our test coverage isn\u0027t so great that you can be 100% sure that you caught them all).","commit_id":"7a92606fd927d0543438ac47207fb8d7ec02617c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"71d6aa42bbaaae46d1ba56372b8055caee425b04","unresolved":false,"context_lines":[{"line_number":115,"context_line":"        \"\"\"Determine if volume is encrypted.\"\"\""},{"line_number":116,"context_line":"        return volume.get(\u0027encryption_key_id\u0027) is not None"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    def _get_attachments(self, is_admin, volume):"},{"line_number":119,"context_line":"        \"\"\"Retrieve the attachments of the volume object.\"\"\""},{"line_number":120,"context_line":"        attachments \u003d []"},{"line_number":121,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"bf51134e_c52c794c","line":118,"in_reply_to":"bf51134e_05a4b107","updated":"2020-07-02 13:52:29.000000000","message":"We never pass True or False directly here rather context.is_admin\nIt\u0027s safe but not a candidate to have a default value.\nSuppose if someone uses this method and forgets to pass the is_admin part, then an admin user will never be able to see the host info in that call.\nThis way it is mandatory to pass the is_admin when calling this method.\nI understand with respect to the security fix but i don\u0027t think this variable can have a default value as it is based on the user context and is dynamic.","commit_id":"7a92606fd927d0543438ac47207fb8d7ec02617c"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"6e9c8709e51aeee7c94aa05f20b2133582417823","unresolved":false,"context_lines":[{"line_number":115,"context_line":"        \"\"\"Determine if volume is encrypted.\"\"\""},{"line_number":116,"context_line":"        return volume.get(\u0027encryption_key_id\u0027) is not None"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    def _get_attachments(self, is_admin, volume):"},{"line_number":119,"context_line":"        \"\"\"Retrieve the attachments of the volume object.\"\"\""},{"line_number":120,"context_line":"        attachments \u003d []"},{"line_number":121,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"bf51134e_3245a771","line":118,"in_reply_to":"bf51134e_122ce345","updated":"2020-07-03 15:14:49.000000000","message":"I have seen bugs being introduced in both cases, when we add a default value and when we don\u0027t, and in the second case it\u0027s always because missing or bad unit tests.\n\nIn this case I agree with Rajat. It is OK to add the new parameter and not set a default value because:\n\n- It is a protected variable\n- We are only calling the method in one place in our whole code (L95)\n- It is a necessary parameter to return the right value (having a default could create a bug when we add new code if we forget to pass the parameter).","commit_id":"7a92606fd927d0543438ac47207fb8d7ec02617c"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"3c407201700ff2fc29222cc95f792b7c616e9f8d","unresolved":false,"context_lines":[{"line_number":115,"context_line":"        \"\"\"Determine if volume is encrypted.\"\"\""},{"line_number":116,"context_line":"        return volume.get(\u0027encryption_key_id\u0027) is not None"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    def _get_attachments(self, is_admin, volume):"},{"line_number":119,"context_line":"        \"\"\"Retrieve the attachments of the volume object.\"\"\""},{"line_number":120,"context_line":"        attachments \u003d []"},{"line_number":121,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"bf51134e_05a4b107","line":118,"in_reply_to":"bf51134e_3b72bc7d","updated":"2020-07-02 13:34:56.000000000","message":"That\u0027s fine, but note that is_admin\u003dFalse is the safe option here (i.e., the sensitive info will *not* be exposed).  So the only time you\u0027d see the sensitive info is when the caller has explicitly passed is_admin\u003dTrue.","commit_id":"7a92606fd927d0543438ac47207fb8d7ec02617c"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"abcdbd250cbad0f4e39a16d19f0f04c7115d4ebe","unresolved":false,"context_lines":[{"line_number":115,"context_line":"        \"\"\"Determine if volume is encrypted.\"\"\""},{"line_number":116,"context_line":"        return volume.get(\u0027encryption_key_id\u0027) is not None"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    def _get_attachments(self, is_admin, volume):"},{"line_number":119,"context_line":"        \"\"\"Retrieve the attachments of the volume object.\"\"\""},{"line_number":120,"context_line":"        attachments \u003d []"},{"line_number":121,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"bf51134e_122ce345","line":118,"in_reply_to":"bf51134e_52465b23","updated":"2020-07-03 14:33:03.000000000","message":"++","commit_id":"7a92606fd927d0543438ac47207fb8d7ec02617c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"62b74535173c27b53ea478bffa1d5f825dafaf7f","unresolved":false,"context_lines":[{"line_number":115,"context_line":"        \"\"\"Determine if volume is encrypted.\"\"\""},{"line_number":116,"context_line":"        return volume.get(\u0027encryption_key_id\u0027) is not None"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    def _get_attachments(self, is_admin, volume):"},{"line_number":119,"context_line":"        \"\"\"Retrieve the attachments of the volume object.\"\"\""},{"line_number":120,"context_line":"        attachments \u003d []"},{"line_number":121,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"bf51134e_344f5035","line":118,"in_reply_to":"bf51134e_52465b23","updated":"2020-07-04 07:05:47.000000000","message":"I understand your point for a generalized scenario but in this case\n1) this is a private method so it is highly unlikely that it is used outside of cinder\n2) I did a codesearch[1] and the only usage i see is in cinder on L#95 (atleast in openstack) so i think in this case it is safe to modify the method signature.\n\n[1] http://codesearch.openstack.org/?q\u003d_get_attachments\u0026i\u003dnope\u0026files\u003d\u0026repos\u003d","commit_id":"7a92606fd927d0543438ac47207fb8d7ec02617c"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"36ba61053bbd378e3b7e9f2d34bc87388247e0ce","unresolved":false,"context_lines":[{"line_number":115,"context_line":"        \"\"\"Determine if volume is encrypted.\"\"\""},{"line_number":116,"context_line":"        return volume.get(\u0027encryption_key_id\u0027) is not None"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    def _get_attachments(self, is_admin, volume):"},{"line_number":119,"context_line":"        \"\"\"Retrieve the attachments of the volume object.\"\"\""},{"line_number":120,"context_line":"        attachments \u003d []"},{"line_number":121,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"bf51134e_52465b23","line":118,"in_reply_to":"bf51134e_c52c794c","updated":"2020-07-03 14:28:00.000000000","message":"We\u0027re talking at cross purposes here.  Let me try to explain.\n\nYou are refactoring an existing function to fix a bug.  You don\u0027t want to a cause a regression somewhere else as a side effect of your fix.  Your change makes this function go from 1 required parameter to 2.  So it\u0027s possible that if you don\u0027t catch all instances of calls to this function and change them, you\u0027ll get a TypeError at some point when only 1 parameter is passed, and whatever API call is in process will blow up.  That will be worse than an admin making a call and not seeing the host name.\n\nSo giving the parameter a default value is a way to preserve the existing method signature in a safe way.  My suggestion isn\u0027t about this particular function, it\u0027s a more general point about maintaining a large software project that has less than perfect unit test coverage (i.e., every large software project).\n\nWhen you write a new function, you can decide what parameters are mandatory.  When you are working with existing code (this function has been around since 2012), you need to be a bit more flexible, or you may wind up introducing another problem.","commit_id":"7a92606fd927d0543438ac47207fb8d7ec02617c"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"c27be25c45a26d9250fe8108835be3bb32a79f5a","unresolved":false,"context_lines":[{"line_number":115,"context_line":"        \"\"\"Determine if volume is encrypted.\"\"\""},{"line_number":116,"context_line":"        return volume.get(\u0027encryption_key_id\u0027) is not None"},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    def _get_attachments(self, is_admin, volume):"},{"line_number":119,"context_line":"        \"\"\"Retrieve the attachments of the volume object.\"\"\""},{"line_number":120,"context_line":"        attachments \u003d []"},{"line_number":121,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"bf51134e_3b72bc7d","line":118,"in_reply_to":"bf51134e_e4539f6b","updated":"2020-07-02 07:55:53.000000000","message":"I don\u0027t think assigning a default value is the right thing here. that assumes the _get_attachments call is always from a non-admin.\nWe should handle every call to this method with proper context and currently i only see one.\nI will update this as a second parameter in a new PS.","commit_id":"7a92606fd927d0543438ac47207fb8d7ec02617c"}],"cinder/tests/unit/api/v2/test_volumes.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"14292272e64c6b252cd6b161a7bdb8d38f05e5f6","unresolved":false,"context_lines":[{"line_number":936,"context_line":"        attachment \u003d db.volume_attach(context.get_admin_context(), values)"},{"line_number":937,"context_line":"        db.volume_attached(context.get_admin_context(),"},{"line_number":938,"context_line":"                           attachment[\u0027id\u0027], fake.INSTANCE_ID, fake_host, \u0027/\u0027)"},{"line_number":939,"context_line":"        db.volume_attachment_get(context.get_admin_context(),"},{"line_number":940,"context_line":"                                 attachment[\u0027id\u0027])"},{"line_number":941,"context_line":""},{"line_number":942,"context_line":"        req \u003d fakes.HTTPRequest.blank(\u0027/v2/volumes/detail\u0027)"},{"line_number":943,"context_line":"        res_dict \u003d self.controller.detail(req)"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_265bb12e","line":940,"range":{"start_line":939,"start_character":0,"end_line":940,"end_character":50},"updated":"2020-07-03 08:54:10.000000000","message":"?: why is this necessary?","commit_id":"6e97ac224a4e6255aaed0e23a2f705ac71dfd029"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"62b74535173c27b53ea478bffa1d5f825dafaf7f","unresolved":false,"context_lines":[{"line_number":936,"context_line":"        attachment \u003d db.volume_attach(context.get_admin_context(), values)"},{"line_number":937,"context_line":"        db.volume_attached(context.get_admin_context(),"},{"line_number":938,"context_line":"                           attachment[\u0027id\u0027], fake.INSTANCE_ID, fake_host, \u0027/\u0027)"},{"line_number":939,"context_line":"        db.volume_attachment_get(context.get_admin_context(),"},{"line_number":940,"context_line":"                                 attachment[\u0027id\u0027])"},{"line_number":941,"context_line":""},{"line_number":942,"context_line":"        req \u003d fakes.HTTPRequest.blank(\u0027/v2/volumes/detail\u0027)"},{"line_number":943,"context_line":"        res_dict \u003d self.controller.detail(req)"}],"source_content_type":"text/x-python","patch_set":7,"id":"bf51134e_c67595fc","line":940,"range":{"start_line":939,"start_character":0,"end_line":940,"end_character":50},"in_reply_to":"bf51134e_265bb12e","updated":"2020-07-04 07:05:47.000000000","message":"This was just a confirmation check that the db entry was created successfully. I will remove it in a new PS or a followup, whichever is preferable.","commit_id":"6e97ac224a4e6255aaed0e23a2f705ac71dfd029"}]}