219 lines
8.7 KiB
Plaintext
219 lines
8.7 KiB
Plaintext
{
|
|
"comments": [
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "fb930dbb_012c8476",
|
|
"filename": "manilaclient/v2/shell.py",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 422,
|
|
"author": {
|
|
"id": 9816
|
|
},
|
|
"writtenOn": "2024-03-27T17:10:16Z",
|
|
"side": 1,
|
|
"message": "I wonder if this should be \u0027\u0027 (empty string) instead. The current logic may work in case a user gives\n\n```\n--metadata \"\u0027\u0027\"\n```\n\nrather than\n\n```\n--metadata \u0027\u0027\n```\n\nassuming the outer \u0027\u0027 or \"\" would be stripped by shell.",
|
|
"range": {
|
|
"startLine": 422,
|
|
"startChar": 27,
|
|
"endLine": 422,
|
|
"endChar": 31
|
|
},
|
|
"revId": "af7940a3ea1b0e254d5b5751a69bf6642307115b",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "40717b8f_173f00af",
|
|
"filename": "manilaclient/v2/shell.py",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 422,
|
|
"author": {
|
|
"id": 9816
|
|
},
|
|
"writtenOn": "2024-03-27T17:13:41Z",
|
|
"side": 1,
|
|
"message": "I\u0027m not quite sure how we simulate the strip behavior by shell in unit tests, though",
|
|
"parentUuid": "fb930dbb_012c8476",
|
|
"range": {
|
|
"startLine": 422,
|
|
"startChar": 27,
|
|
"endLine": 422,
|
|
"endChar": 31
|
|
},
|
|
"revId": "af7940a3ea1b0e254d5b5751a69bf6642307115b",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "19a71d9f_118b688c",
|
|
"filename": "manilaclient/v2/shell.py",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 422,
|
|
"author": {
|
|
"id": 36889
|
|
},
|
|
"writtenOn": "2024-03-27T19:18:43Z",
|
|
"side": 1,
|
|
"message": "Hi, from added test:: test_list_filter_by_metadata_with_only_empty_metadata for which command is: list --metadata \u0027\u0027\n\nSo, if we use: list --metadata \u0027 \u0027 --\u003e option\u003d\" \u0027 \u0027 \"\nIf we use: list --metadata \" \" --\u003e option\u003d\u0027 \" \" \u0027\nIf we use: list --metadata \" \u0027 \u0027 \" --\u003e option\u003d\" \" \u0027 \u0027 \" \"\nIf we use: list --metadata \u0027 \" \" \u0027 --\u003e option\u003d\" \u0027 \" \" \u0027 \"\nThis, is how it is behaving using tests.(have added extra space for clarity in reading)",
|
|
"parentUuid": "40717b8f_173f00af",
|
|
"range": {
|
|
"startLine": 422,
|
|
"startChar": 27,
|
|
"endLine": 422,
|
|
"endChar": 31
|
|
},
|
|
"revId": "af7940a3ea1b0e254d5b5751a69bf6642307115b",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "86697448_4e40f5cc",
|
|
"filename": "manilaclient/v2/shell.py",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 422,
|
|
"author": {
|
|
"id": 36889
|
|
},
|
|
"writtenOn": "2024-03-28T12:47:30Z",
|
|
"side": 1,
|
|
"message": "Done",
|
|
"parentUuid": "19a71d9f_118b688c",
|
|
"range": {
|
|
"startLine": 422,
|
|
"startChar": 27,
|
|
"endLine": 422,
|
|
"endChar": 31
|
|
},
|
|
"revId": "af7940a3ea1b0e254d5b5751a69bf6642307115b",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "30207428_efc2ca66",
|
|
"filename": "manilaclient/v2/shell.py",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 422,
|
|
"author": {
|
|
"id": 9816
|
|
},
|
|
"writtenOn": "2024-03-28T14:23:02Z",
|
|
"side": 1,
|
|
"message": "The problem is that the run_command method does not simulate how shell handles \"\" or \u0027\u0027 so what is tested in test_list_filter_by_metadata_with_only_empty_metadata behaves differently so the current tests passing does not mean the usage via shell is actually fixed.",
|
|
"parentUuid": "86697448_4e40f5cc",
|
|
"range": {
|
|
"startLine": 422,
|
|
"startChar": 27,
|
|
"endLine": 422,
|
|
"endChar": 31
|
|
},
|
|
"revId": "af7940a3ea1b0e254d5b5751a69bf6642307115b",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "bc844855_5613991d",
|
|
"filename": "manilaclient/v2/shell.py",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 422,
|
|
"author": {
|
|
"id": 9816
|
|
},
|
|
"writtenOn": "2024-03-28T14:24:12Z",
|
|
"side": 1,
|
|
"message": "I need some time to deploy actual manila env but if you run the modified code actually then I believe you see what I explained above.",
|
|
"parentUuid": "30207428_efc2ca66",
|
|
"range": {
|
|
"startLine": 422,
|
|
"startChar": 27,
|
|
"endLine": 422,
|
|
"endChar": 31
|
|
},
|
|
"revId": "af7940a3ea1b0e254d5b5751a69bf6642307115b",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "e8a3cf0a_df1af127",
|
|
"filename": "manilaclient/v2/shell.py",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 422,
|
|
"author": {
|
|
"id": 36889
|
|
},
|
|
"writtenOn": "2024-03-28T19:45:26Z",
|
|
"side": 1,
|
|
"message": "Thanks Takashi, understood. Actual run might have different behaviour compared to unit-test simulation. I\u0027ll also try to set up above code in devstack and check.",
|
|
"parentUuid": "bc844855_5613991d",
|
|
"range": {
|
|
"startLine": 422,
|
|
"startChar": 27,
|
|
"endLine": 422,
|
|
"endChar": 31
|
|
},
|
|
"revId": "af7940a3ea1b0e254d5b5751a69bf6642307115b",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": true,
|
|
"key": {
|
|
"uuid": "4f10a41c_de7d7af1",
|
|
"filename": "manilaclient/v2/shell.py",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 422,
|
|
"author": {
|
|
"id": 16643
|
|
},
|
|
"writtenOn": "2024-03-28T22:56:56Z",
|
|
"side": 1,
|
|
"message": "I think it\u0027d be a better user experience to allow filtering with --metadata None instead of requiring the use of quotes of any kind.. today, if we used:\n\n```\nmanila list --metadata None\n```\n\nCurrently, if you do this, \n\nwe\u0027re sending a request to manila like:\n\ncurl -i -X GET http://sharedfilesystems.com/share/v2/shares/detail?metadata\u003d%7B%27None%27%3A+None%7D\n\n\nBasically, we\u0027re filtering by metadata key \"None\" and value \"None\".\n\nSo, perhaps here\u0027s what we can do:\n\n\nin the \"_extract_key_value_options\" method and the \"_extract_metadata\" method, add a kwarg (\"allow_empty_key\u003dTrue\"); then the logic of this method becomes:\n\n\nif allow_empty_key\u003dFalse, don\u0027t execute lines 424 through 426; i.e.:\n\n\n```\ndef _extract_key_value_options(args, option_name, allow_empty_key\u003dTrue):\n result_dict \u003d {}\n duplicate_options \u003d []\n\n options \u003d getattr(args, option_name, None)\n\n if options:\n for option in options:\n # unset doesn\u0027t require a val, so we have the if/else\n if \u0027\u003d\u0027 in option:\n (key, value) \u003d option.split(\u0027\u003d\u0027, 1)\n elif allow_empty_key:\n key \u003d option\n value \u003d None\n else:\n # this is an empty key, disallow it\n continue\n\n if key not in result_dict:\n result_dict[key] \u003d value\n else:\n duplicate_options.append(key)\n\n if len(duplicate_options) \u003e 0:\n duplicate_str \u003d \u0027, \u0027.join(duplicate_options)\n msg \u003d \"Following options were duplicated: %s\" % duplicate_str\n raise exceptions.CommandError(msg)\n return result_dict\n```\n\nOnce this is done, you can enhance the `do_list` method to behave like mentioned below:",
|
|
"parentUuid": "bc844855_5613991d",
|
|
"range": {
|
|
"startLine": 422,
|
|
"startChar": 27,
|
|
"endLine": 422,
|
|
"endChar": 31
|
|
},
|
|
"revId": "af7940a3ea1b0e254d5b5751a69bf6642307115b",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
},
|
|
{
|
|
"unresolved": false,
|
|
"key": {
|
|
"uuid": "2026ce1e_015bcffc",
|
|
"filename": "manilaclient/v2/shell.py",
|
|
"patchSetId": 2
|
|
},
|
|
"lineNbr": 422,
|
|
"author": {
|
|
"id": 36889
|
|
},
|
|
"writtenOn": "2024-04-01T07:58:53Z",
|
|
"side": 1,
|
|
"message": "Done",
|
|
"parentUuid": "4f10a41c_de7d7af1",
|
|
"range": {
|
|
"startLine": 422,
|
|
"startChar": 27,
|
|
"endLine": 422,
|
|
"endChar": 31
|
|
},
|
|
"revId": "af7940a3ea1b0e254d5b5751a69bf6642307115b",
|
|
"serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543"
|
|
}
|
|
]
|
|
} |