)]}'
{"bin/swift-get-nodes":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"d57672a6f06313426cbce73f72c0a1e8895c14dd","unresolved":false,"context_lines":[{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    ring \u003d ring_name \u003d None"},{"line_number":60,"context_line":"    if ring_path:"},{"line_number":61,"context_line":"        ring_name \u003d ring_path.rsplit(\u0027/\u0027, 1)[-1].split(\u0027.\u0027, 1)[0]"},{"line_number":62,"context_line":"        ring \u003d Ring(ring_path)"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    try:"}],"source_content_type":"application/octet-stream","patch_set":6,"id":"5a3905b3_db24480e","line":61,"updated":"2017-01-24 07:12:47.000000000","message":"I don\u0027t think this is really covered by tests, and this is how it\u0027s currently spelled - but I think something like\n\n    os.path.basename(ring_path)[:len(\u0027.ring.gz\u0027)]\n\nwould be equivalent and a little more readable\n\nbut this change has already moved a bunch of untested code under test - even if it\u0027d be a good idea to move it - we shouldn\u0027t hold this up for this last bit","commit_id":"4b861334a7927a72886f79facf83663563875e15"},{"author":{"_account_id":20764,"name":"Sachin","email":"psachin@redhat.com","username":"psachin"},"change_message_id":"e1ab76413bf3c389cefedecb8dff6d1d87e50ff6","unresolved":false,"context_lines":[{"line_number":58,"context_line":""},{"line_number":59,"context_line":"    ring \u003d ring_name \u003d None"},{"line_number":60,"context_line":"    if ring_path:"},{"line_number":61,"context_line":"        ring_name \u003d ring_path.rsplit(\u0027/\u0027, 1)[-1].split(\u0027.\u0027, 1)[0]"},{"line_number":62,"context_line":"        ring \u003d Ring(ring_path)"},{"line_number":63,"context_line":""},{"line_number":64,"context_line":"    try:"}],"source_content_type":"application/octet-stream","patch_set":6,"id":"3a461143_f0492332","line":61,"in_reply_to":"5a3905b3_db24480e","updated":"2017-02-06 07:58:12.000000000","message":"Wow! This is Awesome!!!\n\nThis is why object.tar.gz isn\u0027t a valid ring! \n\n;)","commit_id":"4b861334a7927a72886f79facf83663563875e15"},{"author":{"_account_id":9625,"name":"Thiago da Silva","email":"thiagodasilva@gmail.com","username":"thiago"},"change_message_id":"a58dec02abf7ab315cb3f175f266ed21dd884ddd","unresolved":false,"context_lines":[{"line_number":30,"context_line":"    Usage: %prog [-a] \u003cring.gz\u003e \u003caccount\u003e [\u003ccontainer\u003e [\u003cobject\u003e]]"},{"line_number":31,"context_line":"    Or:    %prog [-a] \u003cring.gz\u003e -p partition"},{"line_number":32,"context_line":"    Or:    %prog [-a] -P policy_name \u003caccount\u003e [\u003ccontainer\u003e [\u003cobject\u003e]]"},{"line_number":33,"context_line":"    Or:    %prog [-a] -P policy_name -p partition"},{"line_number":34,"context_line":"    Note: account, container, object can also be a single arg separated by /"},{"line_number":35,"context_line":"    Example:"},{"line_number":36,"context_line":"        $ %prog -a /etc/swift/account.ring.gz MyAccount"}],"source_content_type":"application/octet-stream","patch_set":7,"id":"fa31d9ce_a9431941","line":33,"updated":"2017-02-13 22:24:11.000000000","message":"too bad we don\u0027t show an option here with -d","commit_id":"f713bb135224b45b16f054a9d90448fd0bebce67"},{"author":{"_account_id":9625,"name":"Thiago da Silva","email":"thiagodasilva@gmail.com","username":"thiago"},"change_message_id":"a58dec02abf7ab315cb3f175f266ed21dd884ddd","unresolved":false,"context_lines":[{"line_number":55,"context_line":"        ring_path, args \u003d parse_get_node_args(options, args)"},{"line_number":56,"context_line":"    except InfoSystemExit as e:"},{"line_number":57,"context_line":"        parser.print_help()"},{"line_number":58,"context_line":"        sys.exit(\u0027ERROR: %s\u0027 % e)"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    ring \u003d ring_name \u003d None"},{"line_number":61,"context_line":"    if ring_path:"}],"source_content_type":"application/octet-stream","patch_set":7,"id":"fa31d9ce_6e42076a","line":58,"range":{"start_line":58,"start_character":17,"end_line":58,"end_character":28},"updated":"2017-02-13 22:24:11.000000000","message":"I\u0027d add a blank line here:\nsys.exit(\u0027\\nERROR: %s\u0027 % e)","commit_id":"f713bb135224b45b16f054a9d90448fd0bebce67"}],"swift/cli/info.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"d57672a6f06313426cbce73f72c0a1e8895c14dd","unresolved":false,"context_lines":[{"line_number":53,"context_line":"        if os.path.exists(args[0]) and args[0].endswith(\u0027ring.gz\u0027):"},{"line_number":54,"context_line":"            ring_path \u003d args.pop(0)"},{"line_number":55,"context_line":"        elif args[0].endswith(\u0027ring.gz\u0027):"},{"line_number":56,"context_line":"            raise InfoSystemExit(\u0027Ring file does not exist\u0027)"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    if len(args) \u003d\u003d 1 and \u0027/\u0027 in args[0]:"},{"line_number":59,"context_line":"        args \u003d args[0].strip(\u0027/\u0027).split(\u0027/\u0027, 2)"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a3905b3_1bf7b059","line":56,"updated":"2017-01-24 07:12:47.000000000","message":"so the else block is ... not args[0].endswith(\u0027ring.gz\u0027) ???\n\nI find this more readable:\n\n\tdiff --git a/swift/cli/info.py b/swift/cli/info.py\n\tindex ffd3ee5..8b3f299 100644\n\t--- a/swift/cli/info.py\n\t+++ b/swift/cli/info.py\n\t@@ -49,10 +49,10 @@ def parse_get_node_args(options, args):\n\t\t if options.policy_name:\n\t\t\t if POLICIES.get_by_name(options.policy_name) is None:\n\t\t\t\t raise InfoSystemExit(\u0027No policy named %r\u0027 % options.policy_name)\n\t-    elif args:\n\t-        if os.path.exists(args[0]) and args[0].endswith(\u0027ring.gz\u0027):\n\t+    elif args and args[0].endswith(\u0027ring.gz\u0027):\n\t+        if os.path.exists(args[0]):\n\t\t\t\t ring_path \u003d args.pop(0)\n\t-        elif args[0].endswith(\u0027ring.gz\u0027):\n\t+        else:\n\t\t\t\t raise InfoSystemExit(\u0027Ring file does not exist\u0027)\n\t \n\t\t if len(args) \u003d\u003d 1 and \u0027/\u0027 in args[0]:","commit_id":"4b861334a7927a72886f79facf83663563875e15"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"d57672a6f06313426cbce73f72c0a1e8895c14dd","unresolved":false,"context_lines":[{"line_number":56,"context_line":"            raise InfoSystemExit(\u0027Ring file does not exist\u0027)"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    if len(args) \u003d\u003d 1 and \u0027/\u0027 in args[0]:"},{"line_number":59,"context_line":"        args \u003d args[0].strip(\u0027/\u0027).split(\u0027/\u0027, 2)"},{"line_number":60,"context_line":""},{"line_number":61,"context_line":"    if not ring_path and not options.policy_name:"},{"line_number":62,"context_line":"        raise InfoSystemExit(\u0027Need to specify policy_name or \u003cring.gz\u003e\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a3905b3_fb2e64a3","line":59,"updated":"2017-01-24 07:12:47.000000000","message":"I think this is split is safe and correct even if there is no \u0027/\u0027 in one and only arg (account name)\n\n\tdiff --git a/swift/cli/info.py b/swift/cli/info.py\n\tindex ffd3ee5..b991760 100644\n\t--- a/swift/cli/info.py\n\t+++ b/swift/cli/info.py\n\t@@ -55,7 +55,7 @@ def parse_get_node_args(options, args):\n\t\t\t elif args[0].endswith(\u0027ring.gz\u0027):\n\t\t\t\t raise InfoSystemExit(\u0027Ring file does not exist\u0027)\n\t \n\t-    if len(args) \u003d\u003d 1 and \u0027/\u0027 in args[0]:\n\t+    if len(args) \u003d\u003d 1:\n\t\t\t args \u003d args[0].strip(\u0027/\u0027).split(\u0027/\u0027, 2)\n\t \n\t\t if not ring_path and not options.policy_name:\n\n\nI suppose one could argue it\u0027s unnecessary - getting rid of the second condition doesn\u0027t reduce the cyclomatic complexity - but i\u0027m not sure the __contains__ condition is any faster than the noop string functions.\n\nI just find less booleans to hold in my head more readable.","commit_id":"4b861334a7927a72886f79facf83663563875e15"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"d57672a6f06313426cbce73f72c0a1e8895c14dd","unresolved":false,"context_lines":[{"line_number":63,"context_line":"    elif ring_path and not (args or options.partition):"},{"line_number":64,"context_line":"        raise InfoSystemExit(\u0027No target specified\u0027)"},{"line_number":65,"context_line":"    elif options.policy_name and not (args or options.partition):"},{"line_number":66,"context_line":"        raise InfoSystemExit(\u0027Policy name requires partition\u0027)"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    if len(args) \u003e 3:"},{"line_number":69,"context_line":"        raise InfoSystemExit(\u0027Invalid arguments\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a3905b3_b6fcf3b8","line":66,"updated":"2017-01-24 07:12:47.000000000","message":"hrmm... I think if you moved the \"not (args or options.partition)\" to a separate guard it\u0027d be a little more clear:\n\n\tdiff --git a/swift/cli/info.py b/swift/cli/info.py\n\tindex ffd3ee5..0a3aa4f 100644\n\t--- a/swift/cli/info.py\n\t+++ b/swift/cli/info.py\n\t@@ -60,10 +60,12 @@ def parse_get_node_args(options, args):\n\t \n\t\t if not ring_path and not options.policy_name:\n\t\t\t raise InfoSystemExit(\u0027Need to specify policy_name or \u003cring.gz\u003e\u0027)\n\t-    elif ring_path and not (args or options.partition):\n\t-        raise InfoSystemExit(\u0027No target specified\u0027)\n\t-    elif options.policy_name and not (args or options.partition):\n\t-        raise InfoSystemExit(\u0027Policy name requires partition\u0027)\n\t+\n\t+    if not (args or options.partition):\n\t+        if options.policy_name:\n\t+            raise InfoSystemExit(\u0027Policy name requires partition\u0027)\n\t+        else:\n\t+            raise InfoSystemExit(\u0027No target specified\u0027)\n\t \n\t\t if len(args) \u003e 3:\n\t\t\t raise InfoSystemExit(\u0027Invalid arguments\u0027)","commit_id":"4b861334a7927a72886f79facf83663563875e15"},{"author":{"_account_id":20764,"name":"Sachin","email":"psachin@redhat.com","username":"psachin"},"change_message_id":"e1ab76413bf3c389cefedecb8dff6d1d87e50ff6","unresolved":false,"context_lines":[{"line_number":63,"context_line":"    elif ring_path and not (args or options.partition):"},{"line_number":64,"context_line":"        raise InfoSystemExit(\u0027No target specified\u0027)"},{"line_number":65,"context_line":"    elif options.policy_name and not (args or options.partition):"},{"line_number":66,"context_line":"        raise InfoSystemExit(\u0027Policy name requires partition\u0027)"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    if len(args) \u003e 3:"},{"line_number":69,"context_line":"        raise InfoSystemExit(\u0027Invalid arguments\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3a461143_d08c27dc","line":66,"in_reply_to":"5a3905b3_b6fcf3b8","updated":"2017-02-06 07:58:12.000000000","message":"Yeah! The code is much more readable and closer to what you suggested in first comment.","commit_id":"4b861334a7927a72886f79facf83663563875e15"}],"test/unit/cli/test_info.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"c3e12449827133cfb9c0d575f40a2078dcbd5cca","unresolved":false,"context_lines":[{"line_number":499,"context_line":"        args \u003d \u0027\u0027"},{"line_number":500,"context_line":"        self.assertRaisesMessage(InfoSystemExit,"},{"line_number":501,"context_line":"                                 \u0027policy_name requires partition\u0027,"},{"line_number":502,"context_line":"                                 parse_get_node_args, options, args.split())"},{"line_number":503,"context_line":"        # a c o"},{"line_number":504,"context_line":"        options \u003d Namespace(policy_name\u003dNone)"},{"line_number":505,"context_line":"        args \u003d \u0027a c o\u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a71b18c_798c1fda","line":502,"updated":"2016-12-06 21:28:58.000000000","message":"this seems like it should say the same thing as\n\n\"objects.ring.gz\"\n\nThe user has identified the ring/policy - but not what they want to look up\n\nSomething like \"no target specified\" seems most correct - is there a test for the case where the user just provides the ring but no other arguments?","commit_id":"4b5448845023ea0efba8ad0f4ae10b5a46edb994"},{"author":{"_account_id":20764,"name":"Sachin","email":"psachin@redhat.com","username":"psachin"},"change_message_id":"13c67079021768dd94dc6214d5bbd49295ae9f1f","unresolved":false,"context_lines":[{"line_number":499,"context_line":"        args \u003d \u0027\u0027"},{"line_number":500,"context_line":"        self.assertRaisesMessage(InfoSystemExit,"},{"line_number":501,"context_line":"                                 \u0027policy_name requires partition\u0027,"},{"line_number":502,"context_line":"                                 parse_get_node_args, options, args.split())"},{"line_number":503,"context_line":"        # a c o"},{"line_number":504,"context_line":"        options \u003d Namespace(policy_name\u003dNone)"},{"line_number":505,"context_line":"        args \u003d \u0027a c o\u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a71b18c_9447d94a","line":502,"in_reply_to":"3a71b18c_798c1fda","updated":"2016-12-07 16:17:36.000000000","message":"I agreed, the message is somewhat misleading and has been fixed.\n\nI added tests to cover conditions you referred.\nI have re-ordered tests so that they are easier to review.","commit_id":"4b5448845023ea0efba8ad0f4ae10b5a46edb994"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"3e9883f4394445a35200f3b6ee16f056e3d341ab","unresolved":false,"context_lines":[{"line_number":523,"context_line":"        # objects.ring.gz a/c/o"},{"line_number":524,"context_line":"        options \u003d Namespace(policy_name\u003dNone)"},{"line_number":525,"context_line":"        args \u003d \u0027object.ring.gz a/c/o\u0027"},{"line_number":526,"context_line":"        with mock.patch(\u0027swift.cli.info.os.path.exists\u0027, lambda p: True):"},{"line_number":527,"context_line":"            ring_path, args \u003d parse_get_node_args(options, args.split())"},{"line_number":528,"context_line":"        self.assertEqual(ring_path, \u0027object.ring.gz\u0027)"},{"line_number":529,"context_line":"        self.assertEqual(args, [\u0027a\u0027, \u0027c\u0027, \u0027o\u0027])"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a71b18c_bf577724","line":526,"range":{"start_line":526,"start_character":57,"end_line":526,"end_character":71},"updated":"2016-12-06 22:11:57.000000000","message":"Instead of using a lambda here, you should use mock.patches return_value:\n\n  with mock.patch(\u0027swift.cli.info.os.path.exists\u0027, return_value\u003dTrue):\n\nSame for the values below.","commit_id":"4b5448845023ea0efba8ad0f4ae10b5a46edb994"},{"author":{"_account_id":20764,"name":"Sachin","email":"psachin@redhat.com","username":"psachin"},"change_message_id":"13c67079021768dd94dc6214d5bbd49295ae9f1f","unresolved":false,"context_lines":[{"line_number":523,"context_line":"        # objects.ring.gz a/c/o"},{"line_number":524,"context_line":"        options \u003d Namespace(policy_name\u003dNone)"},{"line_number":525,"context_line":"        args \u003d \u0027object.ring.gz a/c/o\u0027"},{"line_number":526,"context_line":"        with mock.patch(\u0027swift.cli.info.os.path.exists\u0027, lambda p: True):"},{"line_number":527,"context_line":"            ring_path, args \u003d parse_get_node_args(options, args.split())"},{"line_number":528,"context_line":"        self.assertEqual(ring_path, \u0027object.ring.gz\u0027)"},{"line_number":529,"context_line":"        self.assertEqual(args, [\u0027a\u0027, \u0027c\u0027, \u0027o\u0027])"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a71b18c_eb87675e","line":526,"range":{"start_line":526,"start_character":57,"end_line":526,"end_character":71},"in_reply_to":"3a71b18c_bf577724","updated":"2016-12-07 16:17:36.000000000","message":"Done","commit_id":"4b5448845023ea0efba8ad0f4ae10b5a46edb994"},{"author":{"_account_id":7233,"name":"Matthew Oliver","email":"matt@oliver.net.au","username":"mattoliverau"},"change_message_id":"3e9883f4394445a35200f3b6ee16f056e3d341ab","unresolved":false,"context_lines":[{"line_number":545,"context_line":"        options \u003d Namespace(policy_name\u003d\u0027test\u0027, partition\u003d\u00271\u0027)"},{"line_number":546,"context_line":"        args \u003d \u0027\u0027"},{"line_number":547,"context_line":"        ring_path, args \u003d parse_get_node_args(options, args.split())"},{"line_number":548,"context_line":"        self.assertEqual(ring_path, None)"},{"line_number":549,"context_line":"        self.assertEqual(args, [])"},{"line_number":550,"context_line":"        # -P test a c o"},{"line_number":551,"context_line":"        options \u003d Namespace(policy_name\u003d\u0027test\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a71b18c_2a6407b7","line":548,"range":{"start_line":548,"start_character":13,"end_line":548,"end_character":24},"updated":"2016-12-06 22:11:57.000000000","message":"This can be more specific:\n\n  self.assertIsNone(ring_path)\n\nI mention it, because otherwise someone will come past and submit a patch to correct this later.. so might as well do it now :)\n\nThis can be done for the 2 below as well (lines 554 and 560).","commit_id":"4b5448845023ea0efba8ad0f4ae10b5a46edb994"},{"author":{"_account_id":20764,"name":"Sachin","email":"psachin@redhat.com","username":"psachin"},"change_message_id":"13c67079021768dd94dc6214d5bbd49295ae9f1f","unresolved":false,"context_lines":[{"line_number":545,"context_line":"        options \u003d Namespace(policy_name\u003d\u0027test\u0027, partition\u003d\u00271\u0027)"},{"line_number":546,"context_line":"        args \u003d \u0027\u0027"},{"line_number":547,"context_line":"        ring_path, args \u003d parse_get_node_args(options, args.split())"},{"line_number":548,"context_line":"        self.assertEqual(ring_path, None)"},{"line_number":549,"context_line":"        self.assertEqual(args, [])"},{"line_number":550,"context_line":"        # -P test a c o"},{"line_number":551,"context_line":"        options \u003d Namespace(policy_name\u003d\u0027test\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3a71b18c_a829f59c","line":548,"range":{"start_line":548,"start_character":13,"end_line":548,"end_character":24},"in_reply_to":"3a71b18c_2a6407b7","updated":"2016-12-07 16:17:36.000000000","message":"Done","commit_id":"4b5448845023ea0efba8ad0f4ae10b5a46edb994"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"e38cc2d0347477eee21541575584d0d166c8905f","unresolved":false,"context_lines":[{"line_number":557,"context_line":"        # objects.ring.gz a/"},{"line_number":558,"context_line":"        options \u003d Namespace(policy_name\u003dNone)"},{"line_number":559,"context_line":"        args \u003d \u0027object.ring.gz a/\u0027"},{"line_number":560,"context_line":"        with mock.patch(\u0027swift.cli.info.os.path.exists\u0027, mock.patch):"},{"line_number":561,"context_line":"            ring_path, args \u003d parse_get_node_args(options, args.split())"},{"line_number":562,"context_line":"        self.assertEqual(ring_path, \u0027object.ring.gz\u0027)"},{"line_number":563,"context_line":"        self.assertEqual(args, [\u0027a\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a71b18c_0c7d7de7","line":560,"updated":"2016-12-07 17:39:02.000000000","message":"i think... i think this only works by accident.\n\n*if* you give a second argument to mock.patch - it is a replacement for the thing being patched.  With no argument it gets replaced with mock.MagicMock (which is a cool object you should play with in a repl).\n\nthe code *using* the thing that\u0027s being replaced just has one *requirement* (that the thing be callable with 1 posistional argument) and one *behavior* if the return value of the called thing is truthy - it \"exists\" - otherwise - it does not exist.\n\nSo since mock.patch is callale, accepts one argument (or more!) and returns a context manager object which evaluates truthy - this has the effect of making the UUT think object.ring.gz exist and the test passes.\n\nBut that\u0027s nonsense, use instead:\n\n        with mock.patch(\u0027swift.cli.info.os.path.exists\u0027) as exists:\n            exists.return_value \u003d True\n\n^ note in the above \"exists\" is a mock.MagicMock - seriously these things are cool you should read up on that business - will change your life no joke ;)","commit_id":"581ae238c92723a2e28ceb72ca2d2262c20f925f"},{"author":{"_account_id":20764,"name":"Sachin","email":"psachin@redhat.com","username":"psachin"},"change_message_id":"c390c52a0ef70b0aba44051e4fdafd0f958f43c0","unresolved":false,"context_lines":[{"line_number":557,"context_line":"        # objects.ring.gz a/"},{"line_number":558,"context_line":"        options \u003d Namespace(policy_name\u003dNone)"},{"line_number":559,"context_line":"        args \u003d \u0027object.ring.gz a/\u0027"},{"line_number":560,"context_line":"        with mock.patch(\u0027swift.cli.info.os.path.exists\u0027, mock.patch):"},{"line_number":561,"context_line":"            ring_path, args \u003d parse_get_node_args(options, args.split())"},{"line_number":562,"context_line":"        self.assertEqual(ring_path, \u0027object.ring.gz\u0027)"},{"line_number":563,"context_line":"        self.assertEqual(args, [\u0027a\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a71b18c_29dcce17","line":560,"in_reply_to":"3a71b18c_0c7d7de7","updated":"2016-12-08 09:13:19.000000000","message":"Done","commit_id":"581ae238c92723a2e28ceb72ca2d2262c20f925f"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f86c936583b6ff7fb37017b5818c262871621cf0","unresolved":false,"context_lines":[{"line_number":94,"context_line":"            self.assertTrue(msg in str(e),"},{"line_number":95,"context_line":"                            \"Expected %r in %r\" % (msg, str(e)))"},{"line_number":96,"context_line":"            self.assertTrue(isinstance(e, exc),"},{"line_number":97,"context_line":"                            \"Expected %s, got %s\" % (exc, type(e)))"},{"line_number":98,"context_line":""},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"class TestCliInfo(TestCliInfoBase):"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a71b18c_57442373","line":97,"updated":"2016-12-08 19:31:18.000000000","message":"wow, this is *really* bad - if the called function doesn\u0027t raise an exception the test will still pass :\\\n\nthis should change to this:\n\n\tdiff --git a/test/unit/cli/test_info.py b/test/unit/cli/test_info.py\n\tindex 7f06b24..91ea616 100644\n\t--- a/test/unit/cli/test_info.py\n\t+++ b/test/unit/cli/test_info.py\n\t@@ -88,13 +88,9 @@ class TestCliInfoBase(unittest.TestCase):\n\t\t rmtree(os.path.dirname(self.testdir))\n\t \n\t     def assertRaisesMessage(self, exc, msg, func, *args, **kwargs):\n\t-        try:\n\t+        with self.assertRaises(exc) as ctx:\n\t\t     func(*args, **kwargs)\n\t-        except Exception as e:\n\t-            self.assertTrue(msg in str(e),\n\t-                            \"Expected %r in %r\" % (msg, str(e)))\n\t-            self.assertTrue(isinstance(e, exc),\n\t-                            \"Expected %s, got %s\" % (exc, type(e)))\n\t+        self.assertIn(msg, str(ctx.exception))\n\n\noddly, i made the same change on master and none of the *existing* tests failed - but a *ton* of the new tests in this change are failing because the UUT actually returns a valid value and the the error it\u0027s expecting is raised by the print_info function that\u0027s not acctually being exercised by the test.","commit_id":"c3c2532257867486a0818e9ecff3ac67b53b3e85"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f86c936583b6ff7fb37017b5818c262871621cf0","unresolved":false,"context_lines":[{"line_number":538,"context_line":"        args \u003d \u0027object.ring.gz a/c/o/blah/blah\u0027"},{"line_number":539,"context_line":"        self.assertRaisesMessage(InfoSystemExit,"},{"line_number":540,"context_line":"                                 \u0027Invalid arguments\u0027,"},{"line_number":541,"context_line":"                                 parse_get_node_args, options, args.split())"},{"line_number":542,"context_line":""},{"line_number":543,"context_line":"        # object.ring.gz(without any arguments i.e. a c o)"},{"line_number":544,"context_line":"        options \u003d Namespace(policy_name\u003dNone)"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a71b18c_178f0b24","line":541,"updated":"2016-12-08 19:31:18.000000000","message":"thank you for adding this test - it\u0027s acctually highlighting a bug - the following invocations should be equivalent:\n\n  swift-get-nodes /etc/swift/object.ring.gz AUTH_test test junk/test.ring.gz\n\n  swift-get-nodes /etc/swift/object.ring.gz /AUTH_test/test/junk/test.ring.gz\n\n^ This currently produces an error with this change and works on master - it\u0027s a regression :\u0027(\n\n  swift-get-nodes -P default /AUTH_test/test/junk/test.ring.gz\n\n^ This is IMHO unambiguous as to which policy/ring the user selected [1]\n\n  swift-get-nodes -P default AUTH_test test junk/test.ring.gz\n\n... but as it turns out - they produce different results\n\nThey are all selecting the ring for policy-0 and they are all reference the object in the AUTH_test account, in the test container, and the object named \"junk/test.ring.gz\" [1]\n\n1. OTOH, an invocation like\n\n  swift-get-nodes /AUTH_test/test/junk/test.ring.gz\n\n... would obviously be incredibly ambiguous, without a clear indication of the user\u0027s intent it may be difficult to pick out the best error so any will do\n\n2. N.B. that object names can have a \"/\" in them - it\u0027s how swift\u0027s pseudo folder support works, and also that object names can end in any characters even .ring.gz","commit_id":"c3c2532257867486a0818e9ecff3ac67b53b3e85"},{"author":{"_account_id":20764,"name":"Sachin","email":"psachin@redhat.com","username":"psachin"},"change_message_id":"5107066ed27514983b100fae696c9a3b4a484a26","unresolved":false,"context_lines":[{"line_number":538,"context_line":"        args \u003d \u0027object.ring.gz a/c/o/blah/blah\u0027"},{"line_number":539,"context_line":"        self.assertRaisesMessage(InfoSystemExit,"},{"line_number":540,"context_line":"                                 \u0027Invalid arguments\u0027,"},{"line_number":541,"context_line":"                                 parse_get_node_args, options, args.split())"},{"line_number":542,"context_line":""},{"line_number":543,"context_line":"        # object.ring.gz(without any arguments i.e. a c o)"},{"line_number":544,"context_line":"        options \u003d Namespace(policy_name\u003dNone)"}],"source_content_type":"text/x-python","patch_set":4,"id":"1a6eadb0_a5b0018d","line":541,"in_reply_to":"3a71b18c_178f0b24","updated":"2016-12-16 16:25:53.000000000","message":"Done","commit_id":"c3c2532257867486a0818e9ecff3ac67b53b3e85"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f86c936583b6ff7fb37017b5818c262871621cf0","unresolved":false,"context_lines":[{"line_number":565,"context_line":"        args \u003d \u0027a/c/o\u0027"},{"line_number":566,"context_line":"        ring_path, args \u003d parse_get_node_args(options, args.split())"},{"line_number":567,"context_line":"        self.assertIsNone(ring_path)"},{"line_number":568,"context_line":"        self.assertEqual(args, [\u0027a\u0027, \u0027c\u0027, \u0027o\u0027])"},{"line_number":569,"context_line":"        # -P two -p 1 a c o hooya"},{"line_number":570,"context_line":"        options \u003d Namespace(policy_name\u003d\u0027two\u0027, partition\u003d\u00271\u0027)"},{"line_number":571,"context_line":"        args \u003d \u0027a c o hooya\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a71b18c_d74913d2","line":568,"updated":"2016-12-08 19:31:18.000000000","message":"It seems that print info prefers the partition option instead of the object arguments - which seems fine [1] - either is acceptable in my mind as long as it\u0027s deterministic!  \n\n1. an error that requires the user to choose only one or the other of -p or object name would probably be better - but master currently works exactly the same","commit_id":"c3c2532257867486a0818e9ecff3ac67b53b3e85"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f86c936583b6ff7fb37017b5818c262871621cf0","unresolved":false,"context_lines":[{"line_number":571,"context_line":"        args \u003d \u0027a c o hooya\u0027"},{"line_number":572,"context_line":"        self.assertRaisesMessage(InfoSystemExit,"},{"line_number":573,"context_line":"                                 \u0027Invalid arguments\u0027,"},{"line_number":574,"context_line":"                                 parse_get_node_args, options, args.split())"},{"line_number":575,"context_line":""},{"line_number":576,"context_line":"        # Invalid policy"},{"line_number":577,"context_line":"        # -P undefined"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a71b18c_57e7c3e3","line":574,"updated":"2016-12-08 19:31:18.000000000","message":"this is very correct - if the object name contains slashes - those slashes should very much be part of third positional argument!  A forth posistional argument should produce an error (I think this test would be slightly clearer if it dropped the partition option - testing only one thing at a time makes it more obvious where the error comes from)","commit_id":"c3c2532257867486a0818e9ecff3ac67b53b3e85"},{"author":{"_account_id":20764,"name":"Sachin","email":"psachin@redhat.com","username":"psachin"},"change_message_id":"5107066ed27514983b100fae696c9a3b4a484a26","unresolved":false,"context_lines":[{"line_number":571,"context_line":"        args \u003d \u0027a c o hooya\u0027"},{"line_number":572,"context_line":"        self.assertRaisesMessage(InfoSystemExit,"},{"line_number":573,"context_line":"                                 \u0027Invalid arguments\u0027,"},{"line_number":574,"context_line":"                                 parse_get_node_args, options, args.split())"},{"line_number":575,"context_line":""},{"line_number":576,"context_line":"        # Invalid policy"},{"line_number":577,"context_line":"        # -P undefined"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a71b18c_800817db","line":574,"in_reply_to":"3a71b18c_57e7c3e3","updated":"2016-12-16 16:25:53.000000000","message":"Done","commit_id":"c3c2532257867486a0818e9ecff3ac67b53b3e85"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"f86c936583b6ff7fb37017b5818c262871621cf0","unresolved":false,"context_lines":[{"line_number":597,"context_line":"        args \u003d \u0027a\u0027"},{"line_number":598,"context_line":"        self.assertRaisesMessage(InfoSystemExit,"},{"line_number":599,"context_line":"                                 \"No policy named \u0027undefined\u0027\","},{"line_number":600,"context_line":"                                 parse_get_node_args, options, args.split())"},{"line_number":601,"context_line":"        # -P undefined a c"},{"line_number":602,"context_line":"        options \u003d Namespace(policy_name\u003d\u0027undefined\u0027)"},{"line_number":603,"context_line":"        args \u003d \u0027a c\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a71b18c_17bc6b6b","line":600,"updated":"2016-12-08 19:31:18.000000000","message":"gah, i can\u0027t see it - how are these two tests different!?\n\n... wow, found the problem in assertRaisesMessage","commit_id":"c3c2532257867486a0818e9ecff3ac67b53b3e85"},{"author":{"_account_id":20764,"name":"Sachin","email":"psachin@redhat.com","username":"psachin"},"change_message_id":"5107066ed27514983b100fae696c9a3b4a484a26","unresolved":false,"context_lines":[{"line_number":597,"context_line":"        args \u003d \u0027a\u0027"},{"line_number":598,"context_line":"        self.assertRaisesMessage(InfoSystemExit,"},{"line_number":599,"context_line":"                                 \"No policy named \u0027undefined\u0027\","},{"line_number":600,"context_line":"                                 parse_get_node_args, options, args.split())"},{"line_number":601,"context_line":"        # -P undefined a c"},{"line_number":602,"context_line":"        options \u003d Namespace(policy_name\u003d\u0027undefined\u0027)"},{"line_number":603,"context_line":"        args \u003d \u0027a c\u0027"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a71b18c_2a300401","line":600,"in_reply_to":"3a71b18c_17bc6b6b","updated":"2016-12-16 16:25:53.000000000","message":"Hmm..you are right, seem this don\u0027t need to test ring_path \u0026 args if testing for an error message. Done","commit_id":"c3c2532257867486a0818e9ecff3ac67b53b3e85"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"d57672a6f06313426cbce73f72c0a1e8895c14dd","unresolved":false,"context_lines":[{"line_number":90,"context_line":"    def assertRaisesMessage(self, exc, msg, func, *args, **kwargs):"},{"line_number":91,"context_line":"        with self.assertRaises(exc) as ctx:"},{"line_number":92,"context_line":"            func(*args, **kwargs)"},{"line_number":93,"context_line":"        self.assertIn(msg, str(ctx.exception))"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"class TestCliInfo(TestCliInfoBase):"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a3905b3_7b57d4c9","line":93,"updated":"2017-01-24 07:12:47.000000000","message":"I think we should merge this just to get this landmine out of here when func didn\u0027t raise an exception!","commit_id":"4b861334a7927a72886f79facf83663563875e15"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"d57672a6f06313426cbce73f72c0a1e8895c14dd","unresolved":false,"context_lines":[{"line_number":541,"context_line":"        args \u003d \u0027object.ring.gz\u0027"},{"line_number":542,"context_line":"        self.assertRaisesMessage(InfoSystemExit,"},{"line_number":543,"context_line":"                                 \u0027Ring file does not exist\u0027,"},{"line_number":544,"context_line":"                                 parse_get_node_args, options, args.split())"},{"line_number":545,"context_line":""},{"line_number":546,"context_line":"        # Valid policy"},{"line_number":547,"context_line":"        # -P zero"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a3905b3_5be3f8d8","line":544,"updated":"2017-01-24 07:12:47.000000000","message":"I think all the other tests that specify the ring by file mock os.path.exists","commit_id":"4b861334a7927a72886f79facf83663563875e15"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"d57672a6f06313426cbce73f72c0a1e8895c14dd","unresolved":false,"context_lines":[{"line_number":549,"context_line":"        args \u003d \u0027\u0027"},{"line_number":550,"context_line":"        self.assertRaisesMessage(InfoSystemExit,"},{"line_number":551,"context_line":"                                 \u0027Policy name requires partition\u0027,"},{"line_number":552,"context_line":"                                 parse_get_node_args, options, args.split())"},{"line_number":553,"context_line":"        # -P one a/c/o"},{"line_number":554,"context_line":"        options \u003d Namespace(policy_name\u003d\u0027one\u0027, partition\u003dNone)"},{"line_number":555,"context_line":"        args \u003d \u0027a/c/o\u0027"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a3905b3_d6c89f57","line":552,"updated":"2017-01-24 07:12:47.000000000","message":"I never understood why this message is different depending on if you specify the ring via path or policy?\n\nI think always \"No target specified\" is better\n\n\tdiff --git a/test/unit/cli/test_info.py b/test/unit/cli/test_info.py\n\tindex 10eb0d0..835029a 100644\n\t--- a/test/unit/cli/test_info.py\n\t+++ b/test/unit/cli/test_info.py\n\t@@ -548,7 +548,7 @@ No user metadata found in db file\u0027\u0027\u0027 % POLICIES[0].name\n\t\t\t options \u003d Namespace(policy_name\u003d\u0027zero\u0027, partition\u003dNone)\n\t\t\t args \u003d \u0027\u0027\n\t\t\t self.assertRaisesMessage(InfoSystemExit,\n\t-                                 \u0027Policy name requires partition\u0027,\n\t+                                 \u0027No target specified\u0027,\n\t\t\t\t\t\t\t\t\t  parse_get_node_args, options, args.split())\n\t\t\t # -P one a/c/o\n\t\t\t options \u003d Namespace(policy_name\u003d\u0027one\u0027, partition\u003dNone)","commit_id":"4b861334a7927a72886f79facf83663563875e15"},{"author":{"_account_id":20764,"name":"Sachin","email":"psachin@redhat.com","username":"psachin"},"change_message_id":"e1ab76413bf3c389cefedecb8dff6d1d87e50ff6","unresolved":false,"context_lines":[{"line_number":549,"context_line":"        args \u003d \u0027\u0027"},{"line_number":550,"context_line":"        self.assertRaisesMessage(InfoSystemExit,"},{"line_number":551,"context_line":"                                 \u0027Policy name requires partition\u0027,"},{"line_number":552,"context_line":"                                 parse_get_node_args, options, args.split())"},{"line_number":553,"context_line":"        # -P one a/c/o"},{"line_number":554,"context_line":"        options \u003d Namespace(policy_name\u003d\u0027one\u0027, partition\u003dNone)"},{"line_number":555,"context_line":"        args \u003d \u0027a/c/o\u0027"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a430d35_163b959d","line":552,"in_reply_to":"5a3905b3_d6c89f57","updated":"2017-02-06 07:58:12.000000000","message":"Done","commit_id":"4b861334a7927a72886f79facf83663563875e15"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"d57672a6f06313426cbce73f72c0a1e8895c14dd","unresolved":false,"context_lines":[{"line_number":561,"context_line":"        args \u003d \u0027account container photos/cat.jpg\u0027"},{"line_number":562,"context_line":"        ring_path, args \u003d parse_get_node_args(options, args.split())"},{"line_number":563,"context_line":"        self.assertIsNone(ring_path)"},{"line_number":564,"context_line":"        self.assertEqual(args, [\u0027account\u0027, \u0027container\u0027, \u0027photos/cat.jpg\u0027])"},{"line_number":565,"context_line":"        # -P one account/container/photos/cat.jpg"},{"line_number":566,"context_line":"        options \u003d Namespace(policy_name\u003d\u0027one\u0027, partition\u003dNone)"},{"line_number":567,"context_line":"        args \u003d \u0027account/container/photos/cat.jpg\u0027"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a6eadb0_a1a754a5","line":564,"updated":"2017-01-24 07:12:47.000000000","message":"nice!","commit_id":"4b861334a7927a72886f79facf83663563875e15"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"d57672a6f06313426cbce73f72c0a1e8895c14dd","unresolved":false,"context_lines":[{"line_number":573,"context_line":"        args \u003d \u0027account/container/junk/test.ring.gz\u0027"},{"line_number":574,"context_line":"        ring_path, args \u003d parse_get_node_args(options, args.split())"},{"line_number":575,"context_line":"        self.assertIsNone(ring_path)"},{"line_number":576,"context_line":"        self.assertEqual(args, [\u0027account\u0027, \u0027container\u0027, \u0027junk/test.ring.gz\u0027])"},{"line_number":577,"context_line":"        # -P two a c o hooya"},{"line_number":578,"context_line":"        options \u003d Namespace(policy_name\u003d\u0027two\u0027, partition\u003dNone)"},{"line_number":579,"context_line":"        args \u003d \u0027a c o hooya\u0027"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a6eadb0_01af287d","line":576,"updated":"2017-01-24 07:12:47.000000000","message":"awesome!","commit_id":"4b861334a7927a72886f79facf83663563875e15"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"d57672a6f06313426cbce73f72c0a1e8895c14dd","unresolved":false,"context_lines":[{"line_number":579,"context_line":"        args \u003d \u0027a c o hooya\u0027"},{"line_number":580,"context_line":"        self.assertRaisesMessage(InfoSystemExit,"},{"line_number":581,"context_line":"                                 \u0027Invalid arguments\u0027,"},{"line_number":582,"context_line":"                                 parse_get_node_args, options, args.split())"},{"line_number":583,"context_line":"        # -P zero -p 1"},{"line_number":584,"context_line":"        options \u003d Namespace(policy_name\u003d\u0027zero\u0027, partition\u003d\u00271\u0027)"},{"line_number":585,"context_line":"        args \u003d \u0027\u0027"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a6eadb0_41cda064","line":582,"updated":"2017-01-24 07:12:47.000000000","message":"so awesome!","commit_id":"4b861334a7927a72886f79facf83663563875e15"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"d57672a6f06313426cbce73f72c0a1e8895c14dd","unresolved":false,"context_lines":[{"line_number":591,"context_line":"        args \u003d \u0027a/c/o\u0027"},{"line_number":592,"context_line":"        ring_path, args \u003d parse_get_node_args(options, args.split())"},{"line_number":593,"context_line":"        self.assertIsNone(ring_path)"},{"line_number":594,"context_line":"        self.assertEqual(args, [\u0027a\u0027, \u0027c\u0027, \u0027o\u0027])"},{"line_number":595,"context_line":"        # -P two -p 1 a c o hooya"},{"line_number":596,"context_line":"        options \u003d Namespace(policy_name\u003d\u0027two\u0027, partition\u003d\u00271\u0027)"},{"line_number":597,"context_line":"        args \u003d \u0027a c o hooya\u0027"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a6eadb0_a1be74ab","line":594,"updated":"2017-01-24 07:12:47.000000000","message":"sure","commit_id":"4b861334a7927a72886f79facf83663563875e15"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"d57672a6f06313426cbce73f72c0a1e8895c14dd","unresolved":false,"context_lines":[{"line_number":643,"context_line":"        # ring_path, args \u003d parse_get_node_args(options, args.split())"},{"line_number":644,"context_line":"        self.assertRaisesMessage(InfoSystemExit,"},{"line_number":645,"context_line":"                                 \"No policy named \u0027undefined\u0027\","},{"line_number":646,"context_line":"                                 parse_get_node_args, options, args)"},{"line_number":647,"context_line":""},{"line_number":648,"context_line":"        # Mock tests"},{"line_number":649,"context_line":"        # /etc/swift/object.ring.gz(without any arguments i.e. a c o)"}],"source_content_type":"text/x-python","patch_set":6,"id":"1a6eadb0_01fd686d","line":646,"updated":"2017-01-24 07:12:47.000000000","message":"a little excessive, but sure","commit_id":"4b861334a7927a72886f79facf83663563875e15"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"d57672a6f06313426cbce73f72c0a1e8895c14dd","unresolved":false,"context_lines":[{"line_number":672,"context_line":"            ring_path, args \u003d parse_get_node_args(options, args.split())"},{"line_number":673,"context_line":"        self.assertEqual(ring_path, \u0027/etc/swift/object.ring.gz\u0027)"},{"line_number":674,"context_line":"        self.assertEqual(args, [\u0027a\u0027, \u0027c\u0027, \u0027etc/swift/object.ring.gz\u0027])"},{"line_number":675,"context_line":"        # Invalid path as mentioned in BUG#1539275"},{"line_number":676,"context_line":"        # /etc/swift/object.tar.gz account container object"},{"line_number":677,"context_line":"        options \u003d Namespace(policy_name\u003dNone, partition\u003dNone)"},{"line_number":678,"context_line":"        args \u003d \u0027/etc/swift/object.tar.gz account container object\u0027"}],"source_content_type":"text/x-python","patch_set":6,"id":"5a3905b3_fb190423","line":675,"updated":"2017-01-24 07:12:47.000000000","message":"nice comment!  reading the args and assertion I missed it *again*\n\n \u003e Why isn\u0027t object.tar.gz a valid ring!?\n\n;)","commit_id":"4b861334a7927a72886f79facf83663563875e15"}]}
