)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"7727720cade858e13070439b07777eaefbd38d48","unresolved":false,"context_lines":[{"line_number":9,"context_line":"cinder-scheduler uses all the \"capabilities:\" to scope the"},{"line_number":10,"context_line":"cinder-volume. With no \"capabilities:\" prefix set in extra_spec,"},{"line_number":11,"context_line":"it will be no effect from cinder-scheduler to schedule the"},{"line_number":12,"context_line":"cinder-volume, which is not the purpose of the testcase."},{"line_number":13,"context_line":"To address the review comment, This change will not remove but"},{"line_number":14,"context_line":"adds 2 cases for the \"capabilities:\" prefix to volume type key:"},{"line_number":15,"context_line":"extra-spec \"capabilities:volume_backend_name\"."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"7adec928_41edd78a","line":12,"updated":"2014-05-22 11:54:36.000000000","message":"The above explanation seems wrong because current tests which are without the prefix also work find as I explained.","commit_id":"d0659ed9bbeef7ce91b2ca0c4840436d4ddf18e0"}],"tempest/api/volume/admin/test_multi_backend.py":[{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"665b74d44a957f8f235d91f4bdd2327e6844bd5c","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        cls.volume_client \u003d cls.os_adm.volumes_client"},{"line_number":38,"context_line":"        cls.volume_type_id_list \u003d []"},{"line_number":39,"context_line":"        cls.volume_id_list \u003d []"},{"line_number":40,"context_line":"        backends_key \u003d \"capabilities:volume_backend_name\""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        # Volume/Type creation (uses backend1_name)"},{"line_number":43,"context_line":"        type1_name \u003d data_utils.rand_name(\u0027Type-\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAW3%2F%2FVEQ%3D","line":40,"updated":"2014-04-15 10:59:41.000000000","message":"The unit tests of Cinder also use volume_backend_name without prefix capabilities.\n\nhttps://github.com/openstack/cinder/blob/master/cinder/tests/scheduler/test_filter_scheduler.py#L240\n\nI\u0027m not sure this change is necessary.","commit_id":"c66b3b5db412bf6ea82a456b1a50a986736c1edf"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"9bbdc6ee879d56b6a30ce8f3c4aa8a32b298f814","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        cls.volume_client \u003d cls.os_adm.volumes_client"},{"line_number":38,"context_line":"        cls.volume_type_id_list \u003d []"},{"line_number":39,"context_line":"        cls.volume_id_list \u003d []"},{"line_number":40,"context_line":"        backends_key \u003d \"capabilities:volume_backend_name\""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        # Volume/Type creation (uses backend1_name)"},{"line_number":43,"context_line":"        type1_name \u003d data_utils.rand_name(\u0027Type-\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAXH%2F%2FaCg%3D","line":40,"in_reply_to":"AAAAW3%2F%2FUQU%3D","updated":"2014-04-22 02:09:03.000000000","message":"I am not sure I understand your comment correctly. This patch fixes the problem but the way seems wrong. Why should we merge it without it?","commit_id":"c66b3b5db412bf6ea82a456b1a50a986736c1edf"},{"author":{"_account_id":9485,"name":"Jerry Cai","email":"caimin@cn.ibm.com","username":"jerrycai"},"change_message_id":"eb23e7300d6a36ff149ab17892c57fa8fd19ef03","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        cls.volume_client \u003d cls.os_adm.volumes_client"},{"line_number":38,"context_line":"        cls.volume_type_id_list \u003d []"},{"line_number":39,"context_line":"        cls.volume_id_list \u003d []"},{"line_number":40,"context_line":"        backends_key \u003d \"capabilities:volume_backend_name\""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        # Volume/Type creation (uses backend1_name)"},{"line_number":43,"context_line":"        type1_name \u003d data_utils.rand_name(\u0027Type-\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAW3%2F%2FUQU%3D","line":40,"in_reply_to":"AAAAW3%2F%2FVEQ%3D","updated":"2014-04-22 01:54:53.000000000","message":"Yes, thanks. I think that requires a fix. But as the core reivewer has approved this one, do you think it\u0027s okay to create a new one to track that but first merge this? Thanks.","commit_id":"c66b3b5db412bf6ea82a456b1a50a986736c1edf"},{"author":{"_account_id":9485,"name":"Jerry Cai","email":"caimin@cn.ibm.com","username":"jerrycai"},"change_message_id":"bd293788479bfb145bbae9851f00a307d7eb3ab8","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        cls.volume_client \u003d cls.os_adm.volumes_client"},{"line_number":38,"context_line":"        cls.volume_type_id_list \u003d []"},{"line_number":39,"context_line":"        cls.volume_id_list \u003d []"},{"line_number":40,"context_line":"        backends_key \u003d \"capabilities:volume_backend_name\""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        # Volume/Type creation (uses backend1_name)"},{"line_number":43,"context_line":"        type1_name \u003d data_utils.rand_name(\u0027Type-\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAXH%2F%2FOmU%3D","line":40,"in_reply_to":"AAAAXH%2F%2FOnQ%3D","updated":"2014-04-23 07:21:23.000000000","message":"OK agree, let me think of this change. Thanks for the comment.","commit_id":"c66b3b5db412bf6ea82a456b1a50a986736c1edf"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"7d0e8aeefaf734e9765137f5b625ec0b210d8eb6","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        cls.volume_client \u003d cls.os_adm.volumes_client"},{"line_number":38,"context_line":"        cls.volume_type_id_list \u003d []"},{"line_number":39,"context_line":"        cls.volume_id_list \u003d []"},{"line_number":40,"context_line":"        backends_key \u003d \"capabilities:volume_backend_name\""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        # Volume/Type creation (uses backend1_name)"},{"line_number":43,"context_line":"        type1_name \u003d data_utils.rand_name(\u0027Type-\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAXH%2F%2FOnQ%3D","line":40,"in_reply_to":"AAAAXH%2F%2FOns%3D","updated":"2014-04-23 07:18:34.000000000","message":"Yes, that would be nice.","commit_id":"c66b3b5db412bf6ea82a456b1a50a986736c1edf"},{"author":{"_account_id":9485,"name":"Jerry Cai","email":"caimin@cn.ibm.com","username":"jerrycai"},"change_message_id":"94f4f06825d139dc5de2369ef7a1447e85b14267","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        cls.volume_client \u003d cls.os_adm.volumes_client"},{"line_number":38,"context_line":"        cls.volume_type_id_list \u003d []"},{"line_number":39,"context_line":"        cls.volume_id_list \u003d []"},{"line_number":40,"context_line":"        backends_key \u003d \"capabilities:volume_backend_name\""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        # Volume/Type creation (uses backend1_name)"},{"line_number":43,"context_line":"        type1_name \u003d data_utils.rand_name(\u0027Type-\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAXH%2F%2FOns%3D","line":40,"in_reply_to":"AAAAXH%2F%2FPr8%3D","updated":"2014-04-23 07:16:23.000000000","message":"ok, so your idea is to keep this testcase and add a new one with the \"capabilities:\" prefix. Am I correct?","commit_id":"c66b3b5db412bf6ea82a456b1a50a986736c1edf"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"aa6a5d9eae03452c97852dfd6db344a91d048daf","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        cls.volume_client \u003d cls.os_adm.volumes_client"},{"line_number":38,"context_line":"        cls.volume_type_id_list \u003d []"},{"line_number":39,"context_line":"        cls.volume_id_list \u003d []"},{"line_number":40,"context_line":"        backends_key \u003d \"capabilities:volume_backend_name\""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        # Volume/Type creation (uses backend1_name)"},{"line_number":43,"context_line":"        type1_name \u003d data_utils.rand_name(\u0027Type-\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAXH%2F%2FPr8%3D","line":40,"in_reply_to":"AAAAXH%2F%2FPu4%3D","updated":"2014-04-23 03:44:28.000000000","message":"\u003e it\u0027s no effect to set the extra_spec key without such prefix\n\nThe above comment seems you misunderstand my previous comment.\nIn the following code of cinder, extra_spec works fine with/without the prefix.\n\n 37         for key, req in six.iteritems(extra_specs):\n 38             # Either not scope format, or in capabilities scope\n 39             scope \u003d key.split(\u0027:\u0027)\n 40             if len(scope) \u003e 1 and scope[0] !\u003d \"capabilities\":\n 41                 continue\n 42             elif scope[0] \u003d\u003d \"capabilities\":\n 43                 del scope[0]\n\nIf extra_spec without the prefix \"capabilities:\", len(scope) of line40 is 1 and continue of line41 does not run. That means working fine.\n\nex.\n\n \u003e\u003e\u003e scope \u003d \"volume_backend_name\".split(\u0027:\u0027)\n \u003e\u003e\u003e len(scope)\n 1","commit_id":"c66b3b5db412bf6ea82a456b1a50a986736c1edf"},{"author":{"_account_id":9485,"name":"Jerry Cai","email":"caimin@cn.ibm.com","username":"jerrycai"},"change_message_id":"0f386d74b34db38f3a093480841dded00e0e5bef","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        cls.volume_client \u003d cls.os_adm.volumes_client"},{"line_number":38,"context_line":"        cls.volume_type_id_list \u003d []"},{"line_number":39,"context_line":"        cls.volume_id_list \u003d []"},{"line_number":40,"context_line":"        backends_key \u003d \"capabilities:volume_backend_name\""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        # Volume/Type creation (uses backend1_name)"},{"line_number":43,"context_line":"        type1_name \u003d data_utils.rand_name(\u0027Type-\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAXH%2F%2FPu4%3D","line":40,"in_reply_to":"AAAAXH%2F%2FV5k%3D","updated":"2014-04-23 02:45:21.000000000","message":"Got your point. But my opinion:\nIn order to block the backward incompatible changes, set the \"capabilities:\" is the correct way. I know in the previous way passes the testcases but that\u0027s wrong, reason is just as your code pasted above, it\u0027s no effect to set the extra_spec key without such prefix. And this patch is to fix this wrong, what\u0027s your idea?","commit_id":"c66b3b5db412bf6ea82a456b1a50a986736c1edf"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"40190a50b727eebfdccdde0938f25192288536f7","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        cls.volume_client \u003d cls.os_adm.volumes_client"},{"line_number":38,"context_line":"        cls.volume_type_id_list \u003d []"},{"line_number":39,"context_line":"        cls.volume_id_list \u003d []"},{"line_number":40,"context_line":"        backends_key \u003d \"capabilities:volume_backend_name\""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        # Volume/Type creation (uses backend1_name)"},{"line_number":43,"context_line":"        type1_name \u003d data_utils.rand_name(\u0027Type-\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAXH%2F%2FV5k%3D","line":40,"in_reply_to":"AAAAXH%2F%2FWIw%3D","updated":"2014-04-22 12:20:11.000000000","message":"How about adding this as the other test case?\n\nNow clients can specify volume_backend_name without the prefix \"capabilities:\" also. and current change remove the existing test, so that means Tempest would not block the backward incompatible changes.","commit_id":"c66b3b5db412bf6ea82a456b1a50a986736c1edf"},{"author":{"_account_id":9485,"name":"Jerry Cai","email":"caimin@cn.ibm.com","username":"jerrycai"},"change_message_id":"87bd15cb190b3efae2091482784d14628c8e601e","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        cls.volume_client \u003d cls.os_adm.volumes_client"},{"line_number":38,"context_line":"        cls.volume_type_id_list \u003d []"},{"line_number":39,"context_line":"        cls.volume_id_list \u003d []"},{"line_number":40,"context_line":"        backends_key \u003d \"capabilities:volume_backend_name\""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        # Volume/Type creation (uses backend1_name)"},{"line_number":43,"context_line":"        type1_name \u003d data_utils.rand_name(\u0027Type-\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAXH%2F%2FWIw%3D","line":40,"in_reply_to":"AAAAXH%2F%2FXDI%3D","updated":"2014-04-22 12:02:34.000000000","message":"\u003e\u003eNow extra_spec without \"capabilities:\" also work fine,\nYes, correct\n\u003e\u003ebut these extra_spec will be ignored in the future.\nNo, I mean to test for the VolumeMultiBackendTest, only extra_spec with prefix \"capabilities:\" that is meanful. So need to add this then verify.","commit_id":"c66b3b5db412bf6ea82a456b1a50a986736c1edf"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"d7176d3193f4034cb7e08e2abaf196eb073d93bf","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        cls.volume_client \u003d cls.os_adm.volumes_client"},{"line_number":38,"context_line":"        cls.volume_type_id_list \u003d []"},{"line_number":39,"context_line":"        cls.volume_id_list \u003d []"},{"line_number":40,"context_line":"        backends_key \u003d \"capabilities:volume_backend_name\""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        # Volume/Type creation (uses backend1_name)"},{"line_number":43,"context_line":"        type1_name \u003d data_utils.rand_name(\u0027Type-\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAXH%2F%2FXDI%3D","line":40,"in_reply_to":"AAAAXH%2F%2FXOQ%3D","updated":"2014-04-22 10:07:33.000000000","message":"\u003e I know it will be ignored if the extra spec key doesn\u0027t has the prefix \"capabilities:\"\n\nDo you mean\n\n Now extra_spec without \"capabilities:\" also work fine,\n but these extra_spec will be ignored in the future.\n\n?","commit_id":"c66b3b5db412bf6ea82a456b1a50a986736c1edf"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"43638258fe1d9d287e0cab3545b876c515e133ce","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        cls.volume_client \u003d cls.os_adm.volumes_client"},{"line_number":38,"context_line":"        cls.volume_type_id_list \u003d []"},{"line_number":39,"context_line":"        cls.volume_id_list \u003d []"},{"line_number":40,"context_line":"        backends_key \u003d \"capabilities:volume_backend_name\""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        # Volume/Type creation (uses backend1_name)"},{"line_number":43,"context_line":"        type1_name \u003d data_utils.rand_name(\u0027Type-\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAXH%2F%2FZ9Q%3D","line":40,"in_reply_to":"AAAAXH%2F%2FZ%2Bo%3D","updated":"2014-04-22 02:27:48.000000000","message":"Do you mean this patch is right and there is a bug in Cinder side?\n\nIf so, could you point out the reason why this patch is right?","commit_id":"c66b3b5db412bf6ea82a456b1a50a986736c1edf"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"726ca602c338562501aea624315053f7781b9a2c","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        cls.volume_client \u003d cls.os_adm.volumes_client"},{"line_number":38,"context_line":"        cls.volume_type_id_list \u003d []"},{"line_number":39,"context_line":"        cls.volume_id_list \u003d []"},{"line_number":40,"context_line":"        backends_key \u003d \"capabilities:volume_backend_name\""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        # Volume/Type creation (uses backend1_name)"},{"line_number":43,"context_line":"        type1_name \u003d data_utils.rand_name(\u0027Type-\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAXH%2F%2FZmk%3D","line":40,"in_reply_to":"AAAAXH%2F%2FZ38%3D","updated":"2014-04-22 05:29:31.000000000","message":"Thanks for your explanation, I guess the related parser of cinder is https://github.com/openstack/cinder/blob/master/cinder/openstack/common/scheduler/filters/capabilities_filter.py#L40 .\n\n 29     def _satisfies_extra_specs(self, capabilities, resource_type):\n 30         \"\"\"Check that the capabilities provided by the services satisfy\n 31         the extra specs associated with the resource type.\n 32         \"\"\"\n 33         extra_specs \u003d resource_type.get(\u0027extra_specs\u0027, [])\n 34         if not extra_specs:\n 35             return True\n 36\n 37         for key, req in six.iteritems(extra_specs):\n 38             # Either not scope format, or in capabilities scope\n 39             scope \u003d key.split(\u0027:\u0027)\n 40             if len(scope) \u003e 1 and scope[0] !\u003d \"capabilities\":\n 41                 continue\n 42             elif scope[0] \u003d\u003d \"capabilities\":\n 43                 del scope[0]\n 44\n 45             cap \u003d capabilities\n 46             for index in range(len(scope)):\n 47                 try:\n 48                     cap \u003d cap.get(scope[index], None)\n 49                 except AttributeError:\n 50                     return False\n 51                 if cap is None:\n 52                     return False\n 53             if not extra_specs_ops.match(cap, req):\n 54                 LOG.debug(_(\"extra_spec requirement \u0027%(req)s\u0027 does not match \"\n 55                           \"\u0027%(cap)s\u0027\"), {\u0027req\u0027: req, \u0027cap\u0027: cap})\n 56                 return False\n 57         return True\n\nIn this code, even if not passing the prefix \"capabilities:\" to a key, it would work correctly because it does not match the condition of line 40 like\n\n \u003e\u003e\u003e scope \u003d \"volume_backend_name\".split(\u0027:\u0027)\n \u003e\u003e\u003e len(scope)\n 1\n\nI am sorry if I miss something. I\u0027d like to know why you create this patch. Do you faced a real problem related to this?","commit_id":"c66b3b5db412bf6ea82a456b1a50a986736c1edf"},{"author":{"_account_id":9485,"name":"Jerry Cai","email":"caimin@cn.ibm.com","username":"jerrycai"},"change_message_id":"0228652c7e1a8aa49cca443fef822fd1199ad044","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        cls.volume_client \u003d cls.os_adm.volumes_client"},{"line_number":38,"context_line":"        cls.volume_type_id_list \u003d []"},{"line_number":39,"context_line":"        cls.volume_id_list \u003d []"},{"line_number":40,"context_line":"        backends_key \u003d \"capabilities:volume_backend_name\""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        # Volume/Type creation (uses backend1_name)"},{"line_number":43,"context_line":"        type1_name \u003d data_utils.rand_name(\u0027Type-\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAXH%2F%2FZ38%3D","line":40,"in_reply_to":"AAAAXH%2F%2FZ9Q%3D","updated":"2014-04-22 02:52:04.000000000","message":"I mean this fix patch is right because cinder-scheduler uses all the \"capabilities:\" to scope the cinder-volume. With no \"capabilities:\" prefix set in extra_spec, it will be no effect from cinder-scheduler to schedule the cinder-volume, which is not the purpose of the testcase.\n\nAnd I think the unit tests of Cinder are also wrong in your first comment: https://github.com/openstack/cinder/blob/master/cinder/tests/scheduler/test_filter_scheduler.py#L240, but I will not fix that with this bug due to the reason I said above. I will open a new bug to fix that. Do you agree?","commit_id":"c66b3b5db412bf6ea82a456b1a50a986736c1edf"},{"author":{"_account_id":9485,"name":"Jerry Cai","email":"caimin@cn.ibm.com","username":"jerrycai"},"change_message_id":"8f1096c4cf0a1fe962ee87d5a8c0fee47bdb4f97","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        cls.volume_client \u003d cls.os_adm.volumes_client"},{"line_number":38,"context_line":"        cls.volume_type_id_list \u003d []"},{"line_number":39,"context_line":"        cls.volume_id_list \u003d []"},{"line_number":40,"context_line":"        backends_key \u003d \"capabilities:volume_backend_name\""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        # Volume/Type creation (uses backend1_name)"},{"line_number":43,"context_line":"        type1_name \u003d data_utils.rand_name(\u0027Type-\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAXH%2F%2FXOQ%3D","line":40,"in_reply_to":"AAAAXH%2F%2FZmk%3D","updated":"2014-04-22 09:49:06.000000000","message":"Thanks for comment. I know it will be ignored if the extra spec key doesn\u0027t has the prefix \"capabilities:\".\nBut in this testcase: VolumeMultiBackendTest, it is necessary to set the prefix \"capabilities:\" to test with, that\u0027s the purpose of this testcase. In another word, it would be meaningless if no such prefix got set for such testcase.","commit_id":"c66b3b5db412bf6ea82a456b1a50a986736c1edf"},{"author":{"_account_id":9485,"name":"Jerry Cai","email":"caimin@cn.ibm.com","username":"jerrycai"},"change_message_id":"0c18f2b140f98e363ef04d72311a6069f609ad23","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        cls.volume_client \u003d cls.os_adm.volumes_client"},{"line_number":38,"context_line":"        cls.volume_type_id_list \u003d []"},{"line_number":39,"context_line":"        cls.volume_id_list \u003d []"},{"line_number":40,"context_line":"        backends_key \u003d \"capabilities:volume_backend_name\""},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        # Volume/Type creation (uses backend1_name)"},{"line_number":43,"context_line":"        type1_name \u003d data_utils.rand_name(\u0027Type-\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAXH%2F%2FZ%2Bo%3D","line":40,"in_reply_to":"AAAAXH%2F%2FaCg%3D","updated":"2014-04-22 02:22:44.000000000","message":"No, that\u0027s not I meant. \nYou said it still needs a fix in test_filter_scheduler.py#L240, I checked and agree of that, but not in this fix because finally many other reviewers includes core reviewer have approved this I don\u0027t like to make a new changeset and re-do this process.\nMy suggestion was to open a new bug right now to address your comment, do you think it\u0027s ok? Thanks.","commit_id":"c66b3b5db412bf6ea82a456b1a50a986736c1edf"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"7727720cade858e13070439b07777eaefbd38d48","unresolved":false,"context_lines":[{"line_number":88,"context_line":"    @test.attr(type\u003d\u0027smoke\u0027)"},{"line_number":89,"context_line":"    def test_backend_name_reporting(self):"},{"line_number":90,"context_line":"        # get volume id which created by type without prefix"},{"line_number":91,"context_line":"        volume_id \u003d self.volume_id_list[0]"},{"line_number":92,"context_line":"        self._test_backend_name_reporting_by_volume_id(volume_id)"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    @test.attr(type\u003d\u0027smoke\u0027)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7adec928_01ccaf34","line":91,"updated":"2014-05-22 11:54:36.000000000","message":"This usage of self.volume_id_list is fragile, because volume_id_list[0] does not represent type without prefix. It seems a little less-readable.\n\nSo how about setting volume_id_without_prefix and volume_id_with_prefix valuables in setUpClass() and using them here?","commit_id":"d0659ed9bbeef7ce91b2ca0c4840436d4ddf18e0"},{"author":{"_account_id":6167,"name":"Ken\u0027ichi Ohmichi","email":"ken1ohmichi@gmail.com","username":"oomichi"},"change_message_id":"9251d01c7ca5c87028d3ba98630a4b2ee2808944","unresolved":false,"context_lines":[{"line_number":72,"context_line":"            self.volume[\u0027id\u0027], \u0027available\u0027)"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"    @classmethod"},{"line_number":75,"context_line":"    def _create_type_and_volume_without_prefix(self, backend_name_key):"},{"line_number":76,"context_line":"        # Volume/Type creation"},{"line_number":77,"context_line":"        type_name \u003d data_utils.rand_name(\u0027Type-\u0027)"},{"line_number":78,"context_line":"        vol_name \u003d data_utils.rand_name(\u0027Volume-\u0027)"}],"source_content_type":"text/x-python","patch_set":5,"id":"1ae5cdf2_c715a44a","line":75,"updated":"2014-06-16 12:26:26.000000000","message":"_create_type_and_volume_with_prefix() and _create_type_and_volume_without_prefix() are almost same, so how about merging them into a single method?\n\neg:\n \n _create_type_and_volume(self, backend_name_key)\n     # Volume/Type creation \n     [..]\n     resp, volume \u003d self.volume_client.create_volume(\n         size\u003d1, display_name\u003dvol_name, volume_type\u003dtype_name)\n     self.volume_client.wait_for_volume_status(\n         self.volume[\u0027id\u0027], \u0027available\u0027)\n     return volume[\u0027id\u0027]\n\nand the caller stores volume[\u0027id\u0027] into cls.volume_id_list_with_prefix or cls.volume_id_list_without_prefix.","commit_id":"bedf847f985813cba988c17ad0785a0704ca7d6c"},{"author":{"_account_id":7428,"name":"Yaroslav Lobankov","email":"lobankovyv@gmail.com","username":"ylobankov"},"change_message_id":"3bedd7ffddc6a4a2f449275797dfd60af492708c","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        cls.volume_id_list_without_prefix \u003d []"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"        # Volume/Type creation (uses backend1_name)"},{"line_number":43,"context_line":"        cls._create_type_and_volume(cls, cls.backend1_name, False)"},{"line_number":44,"context_line":"        # Volume/Type creation (uses capabilities:backend1_name)"},{"line_number":45,"context_line":"        backend_with_prefix \u003d \"capabilities:\" + cls.backend1_name"},{"line_number":46,"context_line":"        cls._create_type_and_volume(cls, backend_with_prefix, True)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1ae5cdf2_f99baa14","line":43,"updated":"2014-06-18 16:58:37.000000000","message":"_create_type_and_volume method takes two arguments but you give three.","commit_id":"95ccdfb0e0457471003af5b2034d2fe4fec026be"},{"author":{"_account_id":7139,"name":"Zhi Kun Liu","username":"zhikunliu","inactive":true},"change_message_id":"052862b4f6e478f9e2960b5bc78d723c689ea5fd","unresolved":false,"context_lines":[{"line_number":48,"context_line":"            # Volume/Type creation (uses backend2_name)"},{"line_number":49,"context_line":"            cls._create_type_and_volume(cls.backend2_name, False)"},{"line_number":50,"context_line":"            # Volume/Type creation (uses capabilities:volume_backend_name)"},{"line_number":51,"context_line":"            cls._create_type_and_volume(cls.backend2_name, False)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    @classmethod"},{"line_number":54,"context_line":"    def _create_type_and_volume(self, backend_name_key, with_prefix):"}],"source_content_type":"text/x-python","patch_set":7,"id":"baada198_739b0918","line":51,"updated":"2014-07-14 06:44:35.000000000","message":"False -\u003e True?","commit_id":"1f39ea7344f8a0135506ba17c8e5d7168403f139"},{"author":{"_account_id":9485,"name":"Jerry Cai","email":"caimin@cn.ibm.com","username":"jerrycai"},"change_message_id":"ead90ce67ff756694ee5d802e172845d6678f8da","unresolved":false,"context_lines":[{"line_number":48,"context_line":"            # Volume/Type creation (uses backend2_name)"},{"line_number":49,"context_line":"            cls._create_type_and_volume(cls.backend2_name, False)"},{"line_number":50,"context_line":"            # Volume/Type creation (uses capabilities:volume_backend_name)"},{"line_number":51,"context_line":"            cls._create_type_and_volume(cls.backend2_name, False)"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    @classmethod"},{"line_number":54,"context_line":"    def _create_type_and_volume(self, backend_name_key, with_prefix):"}],"source_content_type":"text/x-python","patch_set":7,"id":"baada198_b6858fd0","line":51,"in_reply_to":"baada198_739b0918","updated":"2014-07-14 07:02:35.000000000","message":"It\u0027s my fault, will fix this.\nAnd for your question, why not remove cases without prefix is per Ken\u0027ichi Ohmichi\u0027s comment on patchset 2, he\u0027d like to keep the original one and just add new one.","commit_id":"1f39ea7344f8a0135506ba17c8e5d7168403f139"},{"author":{"_account_id":7428,"name":"Yaroslav Lobankov","email":"lobankovyv@gmail.com","username":"ylobankov"},"change_message_id":"99d5c2201426f826f907f2c1f71b307f54261d51","unresolved":false,"context_lines":[{"line_number":53,"context_line":"    @classmethod"},{"line_number":54,"context_line":"    def _create_type_and_volume(self, backend_name_key, with_prefix):"},{"line_number":55,"context_line":"        # Volume/Type creation"},{"line_number":56,"context_line":"        type_name \u003d data_utils.rand_name(\u0027Type-\u0027)"},{"line_number":57,"context_line":"        vol_name \u003d data_utils.rand_name(\u0027Volume-\u0027)"},{"line_number":58,"context_line":"        spec_key_with_prefix \u003d \"capabilities:volume_backend_name\""},{"line_number":59,"context_line":"        spec_key_without_prefix \u003d \"volume_backend_name\""}],"source_content_type":"text/x-python","patch_set":7,"id":"baada198_9d845f5f","line":56,"updated":"2014-07-03 13:15:18.000000000","message":"Please, remove dash. \"rand_name\" function adds dash itself.","commit_id":"1f39ea7344f8a0135506ba17c8e5d7168403f139"},{"author":{"_account_id":1921,"name":"Andrea Frittoli","email":"andrea.frittoli@gmail.com","username":"andrea-frittoli"},"change_message_id":"679f129923ea822c660a3e188047123625369e6b","unresolved":false,"context_lines":[{"line_number":79,"context_line":"    def tearDownClass(cls):"},{"line_number":80,"context_line":"        # volumes deletion"},{"line_number":81,"context_line":"        vid_prefix \u003d getattr(cls, \u0027volume_id_list_with_prefix\u0027, [])"},{"line_number":82,"context_line":"        for volume_id in vid_prefix:"},{"line_number":83,"context_line":"            cls.volume_client.delete_volume(volume_id)"},{"line_number":84,"context_line":"            cls.volume_client.wait_for_resource_deletion(volume_id)"},{"line_number":85,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"dab895d6_459b4dc9","line":82,"updated":"2014-06-29 18:49:52.000000000","message":"It would be more efficient to do all the deletes first, and then all the waits. \nThe problem is not introduced by this patch, but it\u0027s made worst by the fact that we have more volumes created now.\nI\u0027d fix this in case you have to make a new patchset for other reasons.","commit_id":"1f39ea7344f8a0135506ba17c8e5d7168403f139"},{"author":{"_account_id":9485,"name":"Jerry Cai","email":"caimin@cn.ibm.com","username":"jerrycai"},"change_message_id":"0a888061dbdb39bdd2fab7f29820b75001fd180a","unresolved":false,"context_lines":[{"line_number":79,"context_line":"    def tearDownClass(cls):"},{"line_number":80,"context_line":"        # volumes deletion"},{"line_number":81,"context_line":"        vid_prefix \u003d getattr(cls, \u0027volume_id_list_with_prefix\u0027, [])"},{"line_number":82,"context_line":"        for volume_id in vid_prefix:"},{"line_number":83,"context_line":"            cls.volume_client.delete_volume(volume_id)"},{"line_number":84,"context_line":"            cls.volume_client.wait_for_resource_deletion(volume_id)"},{"line_number":85,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"baada198_d9f81dc7","line":82,"in_reply_to":"dab895d6_459b4dc9","updated":"2014-07-01 01:11:21.000000000","message":"Thanks Andrea, this is a nice to have fix and not related to this bug purpose directly, so I\u0027d like to leave this in track by another bug. Thank you.","commit_id":"1f39ea7344f8a0135506ba17c8e5d7168403f139"}]}
