Adds --skip-commits to s-m-s-r

This patch replaces --force-commits with a --skip-commits flag in
swift-manage-shard-ranges to determine when to commit object updates.

Change-Id: I6de041f5c12dca2618d22d1271efe242b2f35258
Signed-off-by: Christian Ohanaja <cohanaja@nvidia.com>

swift/cli/manage_shard_ranges.py

@@ -948,6 +948,26 @@ def _add_prompt_args(parser):
              'Cannot be used with --yes option.')
 
 
+def _add_skip_or_force_commits_arg(parser):
+    """
+    We merge in the pending file by default, this is always correct and
+    useful for probe tests where shard containers have unrealistically low
+    numbers of objects, of which a significant proportion may still be in the
+    pending file.  If you have 10GB databases with 100M objects you can use
+    --skip-commits and the selected shard ranges probably won't be that
+    different. The --force-commits option is redundant and may be deprecated.
+    """
+    group = parser.add_mutually_exclusive_group()
+    group.add_argument(
+        '--skip-commits', action='store_true', dest='skip_commits',
+        default=False,
+        help='Skip commits for pending object updates. By default the broker'
+             ' will commit pending object updates.')
+    group.add_argument(
+        '--force-commits', action='store_false', dest='skip_commits',
+        default=argparse.SUPPRESS, help=argparse.SUPPRESS)
+
+
 def _add_max_expanding_arg(parser):
     parser.add_argument(
         '--max-expanding', nargs='?',
@@ -974,13 +994,8 @@ def _make_parser():
                              'expansion_limit')
     parser.add_argument('--verbose', '-v', action='count', default=0,
                         help='Increase output verbosity')
-    # this is useful for probe tests that shard containers with unrealistically
+    _add_skip_or_force_commits_arg(parser)
-    # low numbers of objects, of which a significant proportion may still be in
+
-    # the pending file
-    parser.add_argument(
-        '--force-commits', action='store_true', default=False,
-        help='Force broker to commit pending object updates before finding '
-             'shard ranges. By default the broker will skip commits.')
     subparsers = parser.add_subparsers(
         dest='subcommand', help='Sub-command help', title='Sub-commands')
 
@@ -1185,7 +1200,7 @@ def main(cli_args=None):
     logger = get_logger({}, name='ContainerBroker', log_to_console=True)
     broker = ContainerBroker(os.path.realpath(args.path_to_file),
                              logger=logger,
-                             skip_commits=not args.force_commits)
+                             skip_commits=args.skip_commits)
     try:
         broker.get_info()
     except Exception as exc:

test/unit/cli/test_manage_shard_ranges.py

@@ -160,7 +160,7 @@ class TestManageShardRanges(unittest.TestCase):
                              func=mock.ANY,
                              rows_per_shard=500000,
                              subcommand='find',
-                             force_commits=False,
+                             skip_commits=False,
                              verbose=0,
                              minimum_shard_size=100000)
         mocked.assert_called_once_with(mock.ANY, expected)
@@ -175,7 +175,7 @@ class TestManageShardRanges(unittest.TestCase):
                              func=mock.ANY,
                              rows_per_shard=600,
                              subcommand='find',
-                             force_commits=False,
+                             skip_commits=False,
                              verbose=0,
                              minimum_shard_size=88)
         mocked.assert_called_once_with(mock.ANY, expected)
@@ -191,7 +191,7 @@ class TestManageShardRanges(unittest.TestCase):
                              func=mock.ANY,
                              rows_per_shard=12345,
                              subcommand='find',
-                             force_commits=False,
+                             skip_commits=False,
                              verbose=0,
                              minimum_shard_size=99)
         mocked.assert_called_once_with(mock.ANY, expected)
@@ -205,7 +205,7 @@ class TestManageShardRanges(unittest.TestCase):
                              path_to_file=mock.ANY,
                              func=mock.ANY,
                              subcommand='compact',
-                             force_commits=False,
+                             skip_commits=False,
                              verbose=0,
                              max_expanding=-1,
                              max_shrinking=1,
@@ -224,7 +224,7 @@ class TestManageShardRanges(unittest.TestCase):
                              path_to_file=mock.ANY,
                              func=mock.ANY,
                              subcommand='compact',
-                             force_commits=False,
+                             skip_commits=False,
                              verbose=0,
                              max_expanding=31,
                              max_shrinking=33,
@@ -247,7 +247,7 @@ class TestManageShardRanges(unittest.TestCase):
                              path_to_file=mock.ANY,
                              func=mock.ANY,
                              subcommand='compact',
-                             force_commits=False,
+                             skip_commits=False,
                              verbose=0,
                              max_expanding=11,
                              max_shrinking=22,
@@ -284,7 +284,7 @@ class TestManageShardRanges(unittest.TestCase):
                              path_to_file=mock.ANY,
                              func=mock.ANY,
                              subcommand='compact',
-                             force_commits=False,
+                             skip_commits=False,
                              verbose=0,
                              max_expanding=31,
                              max_shrinking=33,
@@ -317,7 +317,7 @@ class TestManageShardRanges(unittest.TestCase):
                              path_to_file=mock.ANY,
                              func=mock.ANY,
                              subcommand='compact',
-                             force_commits=False,
+                             skip_commits=False,
                              verbose=0,
                              max_expanding=31,
                              max_shrinking=33,
@@ -349,7 +349,7 @@ class TestManageShardRanges(unittest.TestCase):
                              path_to_file=mock.ANY,
                              func=mock.ANY,
                              subcommand='compact',
-                             force_commits=False,
+                             skip_commits=False,
                              verbose=0,
                              max_expanding=31,
                              max_shrinking=33,
@@ -2928,3 +2928,31 @@ class TestManageShardRanges(unittest.TestCase):
         self.assertIn(
             "argument --yes/-y: not allowed with argument --dry-run/-n",
             err_lines[-2], err_lines)
+
+    def test_skip_commits_and_force_commits_args(self):
+        broker = self._make_broker()
+
+        with mock.patch('swift.cli.manage_shard_ranges.find_ranges',
+                        return_value=0) as mocked:
+            ret = main([broker.db_file, '--skip-commits', 'find'])
+        self.assertEqual(0, ret)
+        args = mocked.call_args[0][1]
+        self.assertTrue(args.skip_commits,
+                        '--skip-commits should set skip_commits to True')
+
+        with mock.patch('swift.cli.manage_shard_ranges.find_ranges',
+                        return_value=0) as mocked:
+            ret = main([broker.db_file, '--force-commits', 'find'])
+        self.assertEqual(0, ret)
+        args = mocked.call_args[0][1]
+        self.assertFalse(args.skip_commits,
+                         '--force-commits should set skip_commits to False')
+
+        # default (no flag) sets skip_commits to False
+        with mock.patch('swift.cli.manage_shard_ranges.find_ranges',
+                        return_value=0) as mocked:
+            ret = main([broker.db_file, 'find'])
+        self.assertEqual(0, ret)
+        args = mocked.call_args[0][1]
+        self.assertFalse(args.skip_commits,
+                         'default should set skip_commits to False')