)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9500,"name":"Matt Fischer","email":"matt@mattfischer.com","username":"mfisch"},"change_message_id":"a63304638ebf1253ffe4ae9f0fd57225910c5caa","unresolved":false,"context_lines":[{"line_number":19,"context_line":"that is `batch_size` from the beginning of the result set.  It will then delete"},{"line_number":20,"context_line":"all expired tokens up to that timestamp.  It is possible that this will delete"},{"line_number":21,"context_line":"somewhat more than the `batch_size` number of rows, but it is expect that it"},{"line_number":22,"context_line":"will typicall be close to that number, and will effectively chunk the work into"},{"line_number":23,"context_line":"roughly equal size batches."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Closes-Bug: 1188378"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"1ae5cdf2_96dda772","line":22,"updated":"2014-06-22 22:40:57.000000000","message":"typo: typically","commit_id":"a993c85332ccd824d82057e70a2a4ddce87886ae"},{"author":{"_account_id":10540,"name":"Clayton O\u0027Neill","email":"clayton.oneill@charter.com","username":"coneill"},"change_message_id":"3411555f07ead84d8b1b48af79a1ead61a5fbe3c","unresolved":false,"context_lines":[{"line_number":19,"context_line":"that is `batch_size` from the beginning of the result set.  It will then delete"},{"line_number":20,"context_line":"all expired tokens up to that timestamp.  It is possible that this will delete"},{"line_number":21,"context_line":"somewhat more than the `batch_size` number of rows, but it is expect that it"},{"line_number":22,"context_line":"will typicall be close to that number, and will effectively chunk the work into"},{"line_number":23,"context_line":"roughly equal size batches."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Closes-Bug: 1188378"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"1ae5cdf2_560cffeb","line":22,"in_reply_to":"1ae5cdf2_96dda772","updated":"2014-06-22 23:23:12.000000000","message":"Done","commit_id":"a993c85332ccd824d82057e70a2a4ddce87886ae"},{"author":{"_account_id":11387,"name":"Sam Leong","email":"chio-fai-sam.leong@hpe.com","username":"Chioleong"},"change_message_id":"e8231f09b6961092d2fb219f4a929129193f5999","unresolved":false,"context_lines":[{"line_number":19,"context_line":"the timestamp of one that is `batch_size` from the beginning of the"},{"line_number":20,"context_line":"result set.  It will then delete all expired tokens up to that"},{"line_number":21,"context_line":"timestamp.  It is possible that this will delete somewhat more than the"},{"line_number":22,"context_line":"`batch_size` number of rows, but it is expect that it will typically be"},{"line_number":23,"context_line":"close to that number, and will effectively chunk the work into roughly"},{"line_number":24,"context_line":"equal size batches."},{"line_number":25,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"1ae5cdf2_148166c3","line":22,"updated":"2014-06-24 00:22:05.000000000","message":"it is expected","commit_id":"cf24387a68e69fd66dfb44e330e0931356bc3b20"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"61d386c315bb40fff5a007d9435417ffc4dfc436","unresolved":false,"context_lines":[{"line_number":21,"context_line":"mechanism that does not require subquery support w/LIMIT.  MySQL does"},{"line_number":22,"context_line":"not support DELETE with a subquery that uses LIMIT."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"The batch expiration strategy will will return a timestamp each time"},{"line_number":25,"context_line":"it\u0027s called.  The flush function will then delete all expired tokens up"},{"line_number":26,"context_line":"to that timestamp.  It is possible that this will delete somewhat more"},{"line_number":27,"context_line":"than the `batch_size` number of rows, but it is expected that it will"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"baada198_41ad6183","line":24,"updated":"2014-07-12 20:22:26.000000000","message":"rm will","commit_id":"a67671458e6a5c90b2ba2757f4dca5745ac7cb83"},{"author":{"_account_id":10540,"name":"Clayton O\u0027Neill","email":"clayton.oneill@charter.com","username":"coneill"},"change_message_id":"97bc269007e41c868bf88cb0a86e249d735c2520","unresolved":false,"context_lines":[{"line_number":21,"context_line":"mechanism that does not require subquery support w/LIMIT.  MySQL does"},{"line_number":22,"context_line":"not support DELETE with a subquery that uses LIMIT."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"The batch expiration strategy will will return a timestamp each time"},{"line_number":25,"context_line":"it\u0027s called.  The flush function will then delete all expired tokens up"},{"line_number":26,"context_line":"to that timestamp.  It is possible that this will delete somewhat more"},{"line_number":27,"context_line":"than the `batch_size` number of rows, but it is expected that it will"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"baada198_34f3c9a0","line":24,"in_reply_to":"baada198_41ad6183","updated":"2014-07-16 13:45:33.000000000","message":"Done","commit_id":"a67671458e6a5c90b2ba2757f4dca5745ac7cb83"},{"author":{"_account_id":6486,"name":"Brant Knudson","email":"blk@acm.org","username":"blk-u"},"change_message_id":"58f07801e930841eb1fbc650cb76630943494dfb","unresolved":false,"context_lines":[{"line_number":11,"context_line":"backend.  The batch size method has been replaced with a method that"},{"line_number":12,"context_line":"will return an appropriate iterator for choosing expiration ranges."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"This patch also allows MySQL to use the batch flush functionality to"},{"line_number":15,"context_line":"alleviate problems with Galera replication and large write sets.  Galera"},{"line_number":16,"context_line":"has a modest size limit (1GB) on how large write sets can grow and this"},{"line_number":17,"context_line":"can be reached fairly easily if you run expiration infrequently, or just"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"baada198_8aff9b65","line":14,"updated":"2014-07-19 01:39:30.000000000","message":"suggest split the change to add the mysql strategy out from the refactoring.","commit_id":"1a8a567567ffc9ce85bf31c0a5155915e8275c52"},{"author":{"_account_id":6486,"name":"Brant Knudson","email":"blk@acm.org","username":"blk-u"},"change_message_id":"58f07801e930841eb1fbc650cb76630943494dfb","unresolved":false,"context_lines":[{"line_number":18,"context_line":"have a high token issue rate."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"This also changes the mechanism to do batch flushes to a more portable"},{"line_number":21,"context_line":"mechanism that does not require subquery support w/LIMIT.  MySQL does"},{"line_number":22,"context_line":"not support DELETE with a subquery that uses LIMIT."},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"The batch expiration strategy will return a timestamp each time it\u0027s"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"baada198_0a70eb06","line":21,"updated":"2014-07-19 01:39:30.000000000","message":"We\u0027re already implementing strategies, so I don\u0027t see why we should change the DB2 strategy that\u0027s been working for it. I think we should have a DB2 strategy that works the way it always has and takes advantage of its support for LIMIT (which is more efficient) and a MySQL strategy that doesn\u0027t.","commit_id":"1a8a567567ffc9ce85bf31c0a5155915e8275c52"}],"keystone/tests/test_backend_sql.py":[{"author":{"_account_id":6486,"name":"Brant Knudson","email":"blk@acm.org","username":"blk-u"},"change_message_id":"58f07801e930841eb1fbc650cb76630943494dfb","unresolved":false,"context_lines":[{"line_number":344,"context_line":"            tok.flush_expired_tokens()"},{"line_number":345,"context_line":""},{"line_number":346,"context_line":"        filter_mock \u003d mock_sql.get_session().query().filter()"},{"line_number":347,"context_line":"        self.assertFalse(filter_mock.imit.called)"},{"line_number":348,"context_line":"        self.assertTrue(filter_mock.delete.called_once)"},{"line_number":349,"context_line":""},{"line_number":350,"context_line":"    def test_flush_expired_tokens_batch_mysql(self):"}],"source_content_type":"text/x-python","patch_set":8,"id":"baada198_6a316758","line":347,"updated":"2014-07-19 01:39:30.000000000","message":"why would imit ever be called? Probably meant to be limit.","commit_id":"1a8a567567ffc9ce85bf31c0a5155915e8275c52"},{"author":{"_account_id":10540,"name":"Clayton O\u0027Neill","email":"clayton.oneill@charter.com","username":"coneill"},"change_message_id":"ecb19a0a848bb034c3281cf65817f6e1ce21f1c7","unresolved":false,"context_lines":[{"line_number":344,"context_line":"            tok.flush_expired_tokens()"},{"line_number":345,"context_line":""},{"line_number":346,"context_line":"        filter_mock \u003d mock_sql.get_session().query().filter()"},{"line_number":347,"context_line":"        self.assertFalse(filter_mock.imit.called)"},{"line_number":348,"context_line":"        self.assertTrue(filter_mock.delete.called_once)"},{"line_number":349,"context_line":""},{"line_number":350,"context_line":"    def test_flush_expired_tokens_batch_mysql(self):"}],"source_content_type":"text/x-python","patch_set":8,"id":"baada198_bc401b60","line":347,"in_reply_to":"baada198_6a316758","updated":"2014-07-20 16:07:02.000000000","message":"Done","commit_id":"1a8a567567ffc9ce85bf31c0a5155915e8275c52"},{"author":{"_account_id":6486,"name":"Brant Knudson","email":"blk@acm.org","username":"blk-u"},"change_message_id":"58f07801e930841eb1fbc650cb76630943494dfb","unresolved":false,"context_lines":[{"line_number":364,"context_line":"            # the yield return."},{"line_number":365,"context_line":"            expiry_mock.assert_called_once()"},{"line_number":366,"context_line":"            mock_delete \u003d mock_sql.get_session().query().filter().delete"},{"line_number":367,"context_line":"            self.assertEqual(len(mock_delete.call_args_list), 3)"},{"line_number":368,"context_line":""},{"line_number":369,"context_line":"    def test_expiry_range_batched(self):"},{"line_number":370,"context_line":"        tok \u003d token_sql.Token()"}],"source_content_type":"text/x-python","patch_set":8,"id":"baada198_ca3b5336","line":367,"updated":"2014-07-19 01:39:30.000000000","message":"suggest use testtools\u0027s HashLength matcher it should provide a more useful message if it fails\n\nalso, if this is supposed to be the length of \"[1, 2, 3]\" then it should be more obvious... , e.g., ITERS\u003d[1,2,3] and then make this len(ITERS)","commit_id":"1a8a567567ffc9ce85bf31c0a5155915e8275c52"},{"author":{"_account_id":10540,"name":"Clayton O\u0027Neill","email":"clayton.oneill@charter.com","username":"coneill"},"change_message_id":"ecb19a0a848bb034c3281cf65817f6e1ce21f1c7","unresolved":false,"context_lines":[{"line_number":364,"context_line":"            # the yield return."},{"line_number":365,"context_line":"            expiry_mock.assert_called_once()"},{"line_number":366,"context_line":"            mock_delete \u003d mock_sql.get_session().query().filter().delete"},{"line_number":367,"context_line":"            self.assertEqual(len(mock_delete.call_args_list), 3)"},{"line_number":368,"context_line":""},{"line_number":369,"context_line":"    def test_expiry_range_batched(self):"},{"line_number":370,"context_line":"        tok \u003d token_sql.Token()"}],"source_content_type":"text/x-python","patch_set":8,"id":"baada198_dc45cf6f","line":367,"in_reply_to":"baada198_ca3b5336","updated":"2014-07-20 16:07:02.000000000","message":"Done","commit_id":"1a8a567567ffc9ce85bf31c0a5155915e8275c52"},{"author":{"_account_id":6486,"name":"Brant Knudson","email":"blk@acm.org","username":"blk-u"},"change_message_id":"1c1a09c0ce125193a085c97e4ab0841b40147b91","unresolved":false,"context_lines":[{"line_number":358,"context_line":"            tok \u003d token_sql.Token()"},{"line_number":359,"context_line":"            expiry_mock \u003d mock.Mock()"},{"line_number":360,"context_line":"            ITERS \u003d [1, 2, 3]"},{"line_number":361,"context_line":"            expiry_mock.return_value \u003d iter([1, 2, 3])"},{"line_number":362,"context_line":"            token_sql._expiry_range_batched \u003d expiry_mock"},{"line_number":363,"context_line":"            tok.flush_expired_tokens()"},{"line_number":364,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"baada198_6b305be9","line":361,"updated":"2014-07-28 03:15:33.000000000","message":"user ITERS here rather than [1, 2, 3]","commit_id":"0f4110cd95258713cc81f2ae24adc9aed7129754"},{"author":{"_account_id":10540,"name":"Clayton O\u0027Neill","email":"clayton.oneill@charter.com","username":"coneill"},"change_message_id":"673fd4475e52a3662e73a990a217c928b3d2a4aa","unresolved":false,"context_lines":[{"line_number":358,"context_line":"            tok \u003d token_sql.Token()"},{"line_number":359,"context_line":"            expiry_mock \u003d mock.Mock()"},{"line_number":360,"context_line":"            ITERS \u003d [1, 2, 3]"},{"line_number":361,"context_line":"            expiry_mock.return_value \u003d iter([1, 2, 3])"},{"line_number":362,"context_line":"            token_sql._expiry_range_batched \u003d expiry_mock"},{"line_number":363,"context_line":"            tok.flush_expired_tokens()"},{"line_number":364,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"baada198_c1d93457","line":361,"in_reply_to":"baada198_6b305be9","updated":"2014-07-28 16:43:54.000000000","message":"Done","commit_id":"0f4110cd95258713cc81f2ae24adc9aed7129754"},{"author":{"_account_id":6486,"name":"Brant Knudson","email":"blk@acm.org","username":"blk-u"},"change_message_id":"1c1a09c0ce125193a085c97e4ab0841b40147b91","unresolved":false,"context_lines":[{"line_number":376,"context_line":"        query_mock.one.side_effect \u003d [[\u0027test\u0027], sql.NotFound()]"},{"line_number":377,"context_line":"        for i, x in enumerate(token_sql._expiry_range_batched(sess_mock,"},{"line_number":378,"context_line":"                                                              upper_bound_mock,"},{"line_number":379,"context_line":"                                                              50)):"},{"line_number":380,"context_line":"            if i \u003d\u003d 0:"},{"line_number":381,"context_line":"                # The first time the batch iterator returns, it should return"},{"line_number":382,"context_line":"                # the first result that comes back from the database."}],"source_content_type":"text/x-python","patch_set":11,"id":"baada198_8bde8f68","line":379,"updated":"2014-07-28 03:15:33.000000000","message":"give a name to the 50 so that readers know what it\u0027s being used for. batch_size\u003d50","commit_id":"0f4110cd95258713cc81f2ae24adc9aed7129754"},{"author":{"_account_id":10540,"name":"Clayton O\u0027Neill","email":"clayton.oneill@charter.com","username":"coneill"},"change_message_id":"673fd4475e52a3662e73a990a217c928b3d2a4aa","unresolved":false,"context_lines":[{"line_number":376,"context_line":"        query_mock.one.side_effect \u003d [[\u0027test\u0027], sql.NotFound()]"},{"line_number":377,"context_line":"        for i, x in enumerate(token_sql._expiry_range_batched(sess_mock,"},{"line_number":378,"context_line":"                                                              upper_bound_mock,"},{"line_number":379,"context_line":"                                                              50)):"},{"line_number":380,"context_line":"            if i \u003d\u003d 0:"},{"line_number":381,"context_line":"                # The first time the batch iterator returns, it should return"},{"line_number":382,"context_line":"                # the first result that comes back from the database."}],"source_content_type":"text/x-python","patch_set":11,"id":"baada198_61e868df","line":379,"in_reply_to":"baada198_8bde8f68","updated":"2014-07-28 16:43:54.000000000","message":"Done","commit_id":"0f4110cd95258713cc81f2ae24adc9aed7129754"},{"author":{"_account_id":6486,"name":"Brant Knudson","email":"blk@acm.org","username":"blk-u"},"change_message_id":"1c1a09c0ce125193a085c97e4ab0841b40147b91","unresolved":false,"context_lines":[{"line_number":382,"context_line":"                # the first result that comes back from the database."},{"line_number":383,"context_line":"                self.assertEqual(x, \u0027test\u0027)"},{"line_number":384,"context_line":"            if i \u003d\u003d 1:"},{"line_number":385,"context_line":"                # The section time, the database range function should return"},{"line_number":386,"context_line":"                # nothing, so the batch iterator rewturns the result of the"},{"line_number":387,"context_line":"                # upper_bound function"},{"line_number":388,"context_line":"                self.assertEqual(x, 2)"}],"source_content_type":"text/x-python","patch_set":11,"id":"baada198_cbec072c","line":385,"updated":"2014-07-28 03:15:33.000000000","message":"change \"section\" to \"second\"","commit_id":"0f4110cd95258713cc81f2ae24adc9aed7129754"},{"author":{"_account_id":10540,"name":"Clayton O\u0027Neill","email":"clayton.oneill@charter.com","username":"coneill"},"change_message_id":"673fd4475e52a3662e73a990a217c928b3d2a4aa","unresolved":false,"context_lines":[{"line_number":382,"context_line":"                # the first result that comes back from the database."},{"line_number":383,"context_line":"                self.assertEqual(x, \u0027test\u0027)"},{"line_number":384,"context_line":"            if i \u003d\u003d 1:"},{"line_number":385,"context_line":"                # The section time, the database range function should return"},{"line_number":386,"context_line":"                # nothing, so the batch iterator rewturns the result of the"},{"line_number":387,"context_line":"                # upper_bound function"},{"line_number":388,"context_line":"                self.assertEqual(x, 2)"}],"source_content_type":"text/x-python","patch_set":11,"id":"baada198_01cecc82","line":385,"in_reply_to":"baada198_cbec072c","updated":"2014-07-28 16:43:54.000000000","message":"Done","commit_id":"0f4110cd95258713cc81f2ae24adc9aed7129754"},{"author":{"_account_id":6486,"name":"Brant Knudson","email":"blk@acm.org","username":"blk-u"},"change_message_id":"1c1a09c0ce125193a085c97e4ab0841b40147b91","unresolved":false,"context_lines":[{"line_number":383,"context_line":"                self.assertEqual(x, \u0027test\u0027)"},{"line_number":384,"context_line":"            if i \u003d\u003d 1:"},{"line_number":385,"context_line":"                # The section time, the database range function should return"},{"line_number":386,"context_line":"                # nothing, so the batch iterator rewturns the result of the"},{"line_number":387,"context_line":"                # upper_bound function"},{"line_number":388,"context_line":"                self.assertEqual(x, 2)"},{"line_number":389,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"baada198_ebef0b1b","line":386,"updated":"2014-07-28 03:15:33.000000000","message":"change \"rewturns\" to \"returns\"","commit_id":"0f4110cd95258713cc81f2ae24adc9aed7129754"},{"author":{"_account_id":10540,"name":"Clayton O\u0027Neill","email":"clayton.oneill@charter.com","username":"coneill"},"change_message_id":"673fd4475e52a3662e73a990a217c928b3d2a4aa","unresolved":false,"context_lines":[{"line_number":383,"context_line":"                self.assertEqual(x, \u0027test\u0027)"},{"line_number":384,"context_line":"            if i \u003d\u003d 1:"},{"line_number":385,"context_line":"                # The section time, the database range function should return"},{"line_number":386,"context_line":"                # nothing, so the batch iterator rewturns the result of the"},{"line_number":387,"context_line":"                # upper_bound function"},{"line_number":388,"context_line":"                self.assertEqual(x, 2)"},{"line_number":389,"context_line":""}],"source_content_type":"text/x-python","patch_set":11,"id":"baada198_41c4445f","line":386,"in_reply_to":"baada198_ebef0b1b","updated":"2014-07-28 16:43:54.000000000","message":"Done","commit_id":"0f4110cd95258713cc81f2ae24adc9aed7129754"},{"author":{"_account_id":6486,"name":"Brant Knudson","email":"blk@acm.org","username":"blk-u"},"change_message_id":"1c1a09c0ce125193a085c97e4ab0841b40147b91","unresolved":false,"context_lines":[{"line_number":385,"context_line":"                # The section time, the database range function should return"},{"line_number":386,"context_line":"                # nothing, so the batch iterator rewturns the result of the"},{"line_number":387,"context_line":"                # upper_bound function"},{"line_number":388,"context_line":"                self.assertEqual(x, 2)"},{"line_number":389,"context_line":""},{"line_number":390,"context_line":"    def test_expiry_range_strategy_sqlite(self):"},{"line_number":391,"context_line":"        tok \u003d token_sql.Token()"}],"source_content_type":"text/x-python","patch_set":11,"id":"baada198_2bdd8370","line":388,"updated":"2014-07-28 03:15:33.000000000","message":"if this is supposed to be the same as the 2 on line 372 then use a variable so that it\u0027s obvious.\n\nAlso, there should be assertions for the other values of i, too.","commit_id":"0f4110cd95258713cc81f2ae24adc9aed7129754"},{"author":{"_account_id":10540,"name":"Clayton O\u0027Neill","email":"clayton.oneill@charter.com","username":"coneill"},"change_message_id":"673fd4475e52a3662e73a990a217c928b3d2a4aa","unresolved":false,"context_lines":[{"line_number":385,"context_line":"                # The section time, the database range function should return"},{"line_number":386,"context_line":"                # nothing, so the batch iterator rewturns the result of the"},{"line_number":387,"context_line":"                # upper_bound function"},{"line_number":388,"context_line":"                self.assertEqual(x, 2)"},{"line_number":389,"context_line":""},{"line_number":390,"context_line":"    def test_expiry_range_strategy_sqlite(self):"},{"line_number":391,"context_line":"        tok \u003d token_sql.Token()"}],"source_content_type":"text/x-python","patch_set":11,"id":"baada198_22510bd9","line":388,"in_reply_to":"baada198_2bdd8370","updated":"2014-07-28 16:43:54.000000000","message":"I changed this to be a more meaningful values, so it\u0027s more clear where 2 is coming from, instead of being a magic value.","commit_id":"0f4110cd95258713cc81f2ae24adc9aed7129754"},{"author":{"_account_id":6486,"name":"Brant Knudson","email":"blk@acm.org","username":"blk-u"},"change_message_id":"994541091a4774ac2b6471a3bd1f0a0dfe16e53f","unresolved":false,"context_lines":[{"line_number":370,"context_line":"                            matchers.HasLength(len(ITERS)))"},{"line_number":371,"context_line":""},{"line_number":372,"context_line":"    def test_expiry_range_batched(self):"},{"line_number":373,"context_line":"        upper_bound_mock \u003d mock.Mock(side_effect\u003d[1, \"final value\"])"},{"line_number":374,"context_line":"        sess_mock \u003d mock.Mock()"},{"line_number":375,"context_line":"        query_mock \u003d sess_mock.query().filter().order_by().offset().limit()"},{"line_number":376,"context_line":"        query_mock.one.side_effect \u003d [[\u0027test\u0027], sql.NotFound()]"}],"source_content_type":"text/x-python","patch_set":14,"id":"baada198_991517e8","line":373,"updated":"2014-08-03 15:21:33.000000000","message":"should be consistent and use \u0027 for strings.","commit_id":"465debfa760901f3309fd83b30d796ee6282430b"}],"keystone/token/backends/sql.py":[{"author":{"_account_id":6488,"name":"Clint Byrum","email":"clint@fewbar.com","username":"clint-fewbar"},"change_message_id":"4411382c76cf2152a555d2d863a2486365d6b0ac","unresolved":false,"context_lines":[{"line_number":231,"context_line":"            # in almost all cases.  We\u0027re deleting in batches this way because"},{"line_number":232,"context_line":"            # MySQL doesn\u0027t support DELETE queries with LIMIT subqueries and"},{"line_number":233,"context_line":"            # SQLAlchemy doesn\u0027t support DELETE queries with direct LIMIT"},{"line_number":234,"context_line":"            # clauses."},{"line_number":235,"context_line":"            query \u003d session.query(TokenModel.expires)"},{"line_number":236,"context_line":"            query \u003d query.filter(TokenModel.expires \u003c timeutils.utcnow())"},{"line_number":237,"context_line":"            query \u003d query.offset(batch_size)"}],"source_content_type":"text/x-python","patch_set":4,"id":"1ae5cdf2_60459142","line":234,"updated":"2014-06-25 05:13:01.000000000","message":"\"slightly\" might be understating things. If there is a flood of token creation in a single second, we will be deleting as many tokens as could have been created in a single second.","commit_id":"cf24387a68e69fd66dfb44e330e0931356bc3b20"},{"author":{"_account_id":6488,"name":"Clint Byrum","email":"clint@fewbar.com","username":"clint-fewbar"},"change_message_id":"69ec32be5a5888331583426f5a82830d81df4a85","unresolved":false,"context_lines":[{"line_number":231,"context_line":"            # in almost all cases.  We\u0027re deleting in batches this way because"},{"line_number":232,"context_line":"            # MySQL doesn\u0027t support DELETE queries with LIMIT subqueries and"},{"line_number":233,"context_line":"            # SQLAlchemy doesn\u0027t support DELETE queries with direct LIMIT"},{"line_number":234,"context_line":"            # clauses."},{"line_number":235,"context_line":"            query \u003d session.query(TokenModel.expires)"},{"line_number":236,"context_line":"            query \u003d query.filter(TokenModel.expires \u003c timeutils.utcnow())"},{"line_number":237,"context_line":"            query \u003d query.offset(batch_size)"}],"source_content_type":"text/x-python","patch_set":4,"id":"1ae5cdf2_0e4adb26","line":234,"in_reply_to":"1ae5cdf2_55c71965","updated":"2014-06-25 22:04:33.000000000","message":"I\u0027m actually a little confused now why the code can\u0027t just be kept as-is with batch size defaulting to 100 for all dbs. ??","commit_id":"cf24387a68e69fd66dfb44e330e0931356bc3b20"},{"author":{"_account_id":11816,"name":"mike bayer","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"b800388b7c3d2d1d1e95d993f09b2d76603aa21f","unresolved":false,"context_lines":[{"line_number":231,"context_line":"            # in almost all cases.  We\u0027re deleting in batches this way because"},{"line_number":232,"context_line":"            # MySQL doesn\u0027t support DELETE queries with LIMIT subqueries and"},{"line_number":233,"context_line":"            # SQLAlchemy doesn\u0027t support DELETE queries with direct LIMIT"},{"line_number":234,"context_line":"            # clauses."},{"line_number":235,"context_line":"            query \u003d session.query(TokenModel.expires)"},{"line_number":236,"context_line":"            query \u003d query.filter(TokenModel.expires \u003c timeutils.utcnow())"},{"line_number":237,"context_line":"            query \u003d query.offset(batch_size)"}],"source_content_type":"text/x-python","patch_set":4,"id":"1ae5cdf2_55c71965","line":234,"in_reply_to":"1ae5cdf2_56f731d4","updated":"2014-06-25 18:46:55.000000000","message":"MySQL\u0027s \"DELETE... LIMIT\" thing is totally non-standard SQL.   I think getting this to work in a chunked style is the best way to go as it can be abstracted out.","commit_id":"cf24387a68e69fd66dfb44e330e0931356bc3b20"},{"author":{"_account_id":10540,"name":"Clayton O\u0027Neill","email":"clayton.oneill@charter.com","username":"coneill"},"change_message_id":"600fd87ec277201e4f8e21d90238fa5b3d7786d8","unresolved":false,"context_lines":[{"line_number":231,"context_line":"            # in almost all cases.  We\u0027re deleting in batches this way because"},{"line_number":232,"context_line":"            # MySQL doesn\u0027t support DELETE queries with LIMIT subqueries and"},{"line_number":233,"context_line":"            # SQLAlchemy doesn\u0027t support DELETE queries with direct LIMIT"},{"line_number":234,"context_line":"            # clauses."},{"line_number":235,"context_line":"            query \u003d session.query(TokenModel.expires)"},{"line_number":236,"context_line":"            query \u003d query.filter(TokenModel.expires \u003c timeutils.utcnow())"},{"line_number":237,"context_line":"            query \u003d query.offset(batch_size)"}],"source_content_type":"text/x-python","patch_set":4,"id":"1ae5cdf2_56f731d4","line":234,"in_reply_to":"1ae5cdf2_60459142","updated":"2014-06-25 12:54:30.000000000","message":"I\u0027m not dead set on this approach.  I\u0027d be glad to implement another mechanism if it meets the same goals.  I ended up here because I could not find a way to get SQLAlchemy to allow me to do something like \"DELETE FROM token WHERE expire \u003c NOW() LIMIT \u003cbatchsize\u003e\" without reverting to plain SQL.","commit_id":"cf24387a68e69fd66dfb44e330e0931356bc3b20"},{"author":{"_account_id":11816,"name":"mike bayer","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"6e87e20c0b7b26c5b30ea65896838c76e0d581b6","unresolved":false,"context_lines":[{"line_number":235,"context_line":"            query \u003d session.query(TokenModel.expires)"},{"line_number":236,"context_line":"            query \u003d query.filter(TokenModel.expires \u003c timeutils.utcnow())"},{"line_number":237,"context_line":"            query \u003d query.offset(batch_size)"},{"line_number":238,"context_line":"            query \u003d query.limit(1)"},{"line_number":239,"context_line":"            while True:"},{"line_number":240,"context_line":"                try:"},{"line_number":241,"context_line":"                    next_expiration \u003d query.one()[0]"}],"source_content_type":"text/x-python","patch_set":4,"id":"1ae5cdf2_332cceac","line":238,"updated":"2014-06-23 14:43:17.000000000","message":"so I have a lot of thoughts on this.   I think the specific technique here, considering that we\u0027re targeting MySQL, is the best we can do; that is we have to scan through groups of 1000 rows.   MySQL doesn\u0027t support window functions or a \"row number\" type of function so offset() here is really how we\u0027re getting windows based on a row number, OK.     I would suggest that we add an ORDER BY here, as you can\u0027t assume in all cases that the natural ordering of the table is going to give you what you expect (it might be the case on MySQL and for how this table is populated, but if this code is run against Oracle, who knows):\n\n    query \u003d query.order_by(TokenModel.expires)\n\nBeyond that aspect, if this code were run against Postgresql or another window-function supporting DB, I\u0027d want to use a window to get at the ranges.   Between that, and the way this code \"falls through\" into the \"normal\" delete, i think it would be cleaner if the part of this which grabs batches were factored into an iterator function.   That way, we can swap different iterators based on DB backend, and also this code would be easier to read:\n\n    batch_size \u003d self.token_flush_batch_size(dialect)\n    query \u003d session.query(TokenModel)\n    for expiry_time in self._expiry_range(TokenModel.expires,  \n          timeutils.utcnow, batch_size):\n\t  query.filter(TokenModel.expires \u003c \n                     expiry_time).delete(synchronize_session\u003dFalse)\n\nso that above we only see a single line of code dealing with delete()  and the job of returning the date ranges is handled in a different function.   The iteration and the \"delete\" function can also be tested separately (in the latter case, by patching in a fixed iteration using mock.patch).\n\nnote I\u0027m passing timeutils.utcnow as a callable to the function, so that it can be called repeatedly if necessary in the iteration function; this is assuming that we\u0027re concerned about catching new timestamps entering the result set as time passes.  This can be called up-front if that is not a concern here.\n\nThe iteration function here could later have a version that uses real window functions, based on backend; see https://bitbucket.org/zzzeek/sqlalchemy/wiki/UsageRecipes/WindowedRangeQuery for an example of something similar.","commit_id":"cf24387a68e69fd66dfb44e330e0931356bc3b20"},{"author":{"_account_id":2903,"name":"Morgan Fainberg","email":"morgan.fainberg@gmail.com","username":"mdrnstm"},"change_message_id":"aa4c5f764cce721a42591b01a6dff947dde2a491","unresolved":false,"context_lines":[{"line_number":235,"context_line":"            query \u003d session.query(TokenModel.expires)"},{"line_number":236,"context_line":"            query \u003d query.filter(TokenModel.expires \u003c timeutils.utcnow())"},{"line_number":237,"context_line":"            query \u003d query.offset(batch_size)"},{"line_number":238,"context_line":"            query \u003d query.limit(1)"},{"line_number":239,"context_line":"            while True:"},{"line_number":240,"context_line":"                try:"},{"line_number":241,"context_line":"                    next_expiration \u003d query.one()[0]"}],"source_content_type":"text/x-python","patch_set":4,"id":"1ae5cdf2_c7939fa2","line":238,"in_reply_to":"1ae5cdf2_1a026372","updated":"2014-06-25 04:44:17.000000000","message":"@Michael,\n\nThanks for the input here, the separate function is great feedback.","commit_id":"cf24387a68e69fd66dfb44e330e0931356bc3b20"},{"author":{"_account_id":7725,"name":"David Stanek","email":"dstanek@dstanek.com","username":"dstanek"},"change_message_id":"5b5abe859b62e037f8a791f1fdeeb8f500204e19","unresolved":false,"context_lines":[{"line_number":235,"context_line":"            query \u003d session.query(TokenModel.expires)"},{"line_number":236,"context_line":"            query \u003d query.filter(TokenModel.expires \u003c timeutils.utcnow())"},{"line_number":237,"context_line":"            query \u003d query.offset(batch_size)"},{"line_number":238,"context_line":"            query \u003d query.limit(1)"},{"line_number":239,"context_line":"            while True:"},{"line_number":240,"context_line":"                try:"},{"line_number":241,"context_line":"                    next_expiration \u003d query.one()[0]"}],"source_content_type":"text/x-python","patch_set":4,"id":"1ae5cdf2_6cbe70b0","line":238,"in_reply_to":"1ae5cdf2_332cceac","updated":"2014-06-23 17:54:07.000000000","message":"@Michael: putting the logic for dealing with ranges in a separate function is a great idea.","commit_id":"cf24387a68e69fd66dfb44e330e0931356bc3b20"},{"author":{"_account_id":10540,"name":"Clayton O\u0027Neill","email":"clayton.oneill@charter.com","username":"coneill"},"change_message_id":"4a6952d9034ada9acbff2ace49b71a762c938169","unresolved":false,"context_lines":[{"line_number":235,"context_line":"            query \u003d session.query(TokenModel.expires)"},{"line_number":236,"context_line":"            query \u003d query.filter(TokenModel.expires \u003c timeutils.utcnow())"},{"line_number":237,"context_line":"            query \u003d query.offset(batch_size)"},{"line_number":238,"context_line":"            query \u003d query.limit(1)"},{"line_number":239,"context_line":"            while True:"},{"line_number":240,"context_line":"                try:"},{"line_number":241,"context_line":"                    next_expiration \u003d query.one()[0]"}],"source_content_type":"text/x-python","patch_set":4,"id":"1ae5cdf2_1a026372","line":238,"in_reply_to":"1ae5cdf2_332cceac","updated":"2014-06-24 08:52:18.000000000","message":"This is great feedback, I\u0027ll work on getting a new patch set put together.","commit_id":"cf24387a68e69fd66dfb44e330e0931356bc3b20"},{"author":{"_account_id":10540,"name":"Clayton O\u0027Neill","email":"clayton.oneill@charter.com","username":"coneill"},"change_message_id":"7b713531ac2ea762c302ff6fe346140de7b38671","unresolved":false,"context_lines":[{"line_number":235,"context_line":"            query \u003d session.query(TokenModel.expires)"},{"line_number":236,"context_line":"            query \u003d query.filter(TokenModel.expires \u003c timeutils.utcnow())"},{"line_number":237,"context_line":"            query \u003d query.offset(batch_size)"},{"line_number":238,"context_line":"            query \u003d query.limit(1)"},{"line_number":239,"context_line":"            while True:"},{"line_number":240,"context_line":"                try:"},{"line_number":241,"context_line":"                    next_expiration \u003d query.one()[0]"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_b5622914","line":238,"in_reply_to":"1ae5cdf2_6cbe70b0","updated":"2014-07-18 02:35:31.000000000","message":"Done","commit_id":"cf24387a68e69fd66dfb44e330e0931356bc3b20"},{"author":{"_account_id":6488,"name":"Clint Byrum","email":"clint@fewbar.com","username":"clint-fewbar"},"change_message_id":"4411382c76cf2152a555d2d863a2486365d6b0ac","unresolved":false,"context_lines":[{"line_number":235,"context_line":"            query \u003d session.query(TokenModel.expires)"},{"line_number":236,"context_line":"            query \u003d query.filter(TokenModel.expires \u003c timeutils.utcnow())"},{"line_number":237,"context_line":"            query \u003d query.offset(batch_size)"},{"line_number":238,"context_line":"            query \u003d query.limit(1)"},{"line_number":239,"context_line":"            while True:"},{"line_number":240,"context_line":"                try:"},{"line_number":241,"context_line":"                    next_expiration \u003d query.one()[0]"}],"source_content_type":"text/x-python","patch_set":4,"id":"1ae5cdf2_45c1872e","line":238,"in_reply_to":"1ae5cdf2_6cbe70b0","updated":"2014-06-25 05:13:01.000000000","message":"There is just no great cross-database solution here. Abstracting away the query seems like a way to help control divergent behavior in databases. It sounds like using window functions is a significant optimization for some databases that would be useful to pursue in subsequent patches.\n\nRegarding the time window, I do not think the target cut-off should change as the program runs. Otherwise the behavior would be non-deterministic given a known data-set. In this case, I think we want an admin to be able to predict what the tool will do. If it were to run too slowly for some reason, it might just run forever.","commit_id":"cf24387a68e69fd66dfb44e330e0931356bc3b20"},{"author":{"_account_id":10540,"name":"Clayton O\u0027Neill","email":"clayton.oneill@charter.com","username":"coneill"},"change_message_id":"7b713531ac2ea762c302ff6fe346140de7b38671","unresolved":false,"context_lines":[{"line_number":235,"context_line":"            query \u003d session.query(TokenModel.expires)"},{"line_number":236,"context_line":"            query \u003d query.filter(TokenModel.expires \u003c timeutils.utcnow())"},{"line_number":237,"context_line":"            query \u003d query.offset(batch_size)"},{"line_number":238,"context_line":"            query \u003d query.limit(1)"},{"line_number":239,"context_line":"            while True:"},{"line_number":240,"context_line":"                try:"},{"line_number":241,"context_line":"                    next_expiration \u003d query.one()[0]"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_55aaad1a","line":238,"in_reply_to":"1ae5cdf2_c7939fa2","updated":"2014-07-18 02:35:31.000000000","message":"Done","commit_id":"cf24387a68e69fd66dfb44e330e0931356bc3b20"},{"author":{"_account_id":5046,"name":"Lance Bragstad","email":"lbragstad@redhat.com","username":"ldbragst"},"change_message_id":"61d386c315bb40fff5a007d9435417ffc4dfc436","unresolved":false,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"import copy"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"import functools"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"from keystone.common import sql"},{"line_number":20,"context_line":"from keystone import config"}],"source_content_type":"text/x-python","patch_set":6,"id":"baada198_61e985b6","line":17,"updated":"2014-07-12 20:22:26.000000000","message":"nit: This should be grouped with line 15 since it is a python standard library.","commit_id":"a67671458e6a5c90b2ba2757f4dca5745ac7cb83"},{"author":{"_account_id":10540,"name":"Clayton O\u0027Neill","email":"clayton.oneill@charter.com","username":"coneill"},"change_message_id":"f71a65d079282c985845bb7cbc8eebb48d4d8e55","unresolved":false,"context_lines":[{"line_number":14,"context_line":""},{"line_number":15,"context_line":"import copy"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"import functools"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"from keystone.common import sql"},{"line_number":20,"context_line":"from keystone import config"}],"source_content_type":"text/x-python","patch_set":6,"id":"baada198_b4927949","line":17,"in_reply_to":"baada198_61e985b6","updated":"2014-07-16 13:45:16.000000000","message":"Done","commit_id":"a67671458e6a5c90b2ba2757f4dca5745ac7cb83"},{"author":{"_account_id":2903,"name":"Morgan Fainberg","email":"morgan.fainberg@gmail.com","username":"mdrnstm"},"change_message_id":"3e59ee78362ea902ea3ff7ab19cd9d2d4a08afc3","unresolved":false,"context_lines":[{"line_number":263,"context_line":"                                                  batch_size\u003d1000)"},{"line_number":264,"context_line":"            # We want somewhat more than 100, since Galera replication delay is"},{"line_number":265,"context_line":"            # at least RTT*2.  This can be a significant amount of time if"},{"line_number":266,"context_line":"            # doing replication across a WAN."},{"line_number":267,"context_line":"        return expiry_range_func"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"    def flush_expired_tokens(self):"}],"source_content_type":"text/x-python","patch_set":7,"id":"baada198_75c4ed91","line":266,"updated":"2014-07-18 21:03:01.000000000","message":"I sincerely hope no one is doing keystone token replication across a WAN. But I don\u0027t suppose we shouldn\u0027t guard against breaking things or doing ugly things with this scenario.","commit_id":"cdfbcb2f659c0c2905b0199587995604ceb47ed6"},{"author":{"_account_id":2903,"name":"Morgan Fainberg","email":"morgan.fainberg@gmail.com","username":"mdrnstm"},"change_message_id":"3e59ee78362ea902ea3ff7ab19cd9d2d4a08afc3","unresolved":false,"context_lines":[{"line_number":280,"context_line":"            total_removed +\u003d row_count"},{"line_number":281,"context_line":""},{"line_number":282,"context_line":"        session.flush()"},{"line_number":283,"context_line":"        LOG.info(\"Total expired tokens removed: %d\", total_removed)"}],"source_content_type":"text/x-python","patch_set":7,"id":"baada198_75722d7d","line":283,"updated":"2014-07-18 21:03:01.000000000","message":"Suggest converting this to a debug message, however if Info really is appropriate:\n\nLog info should be (afaik) translated. Please add translation here. you\u0027ll need to import _LI from keystone.i18n and do something like:\n\n    LOG.info(_LI(\u0027Total expired tokens removed: %d\u0027), total_removed)\n\n(NIT): Please prefer using single quotes over double quotes.\n\nBoth of these comments apply to line 278 as well and line 212.","commit_id":"cdfbcb2f659c0c2905b0199587995604ceb47ed6"},{"author":{"_account_id":2903,"name":"Morgan Fainberg","email":"morgan.fainberg@gmail.com","username":"mdrnstm"},"change_message_id":"bad199bbaf0ce782894840e02c746f1d71c5dad5","unresolved":false,"context_lines":[{"line_number":280,"context_line":"            total_removed +\u003d row_count"},{"line_number":281,"context_line":""},{"line_number":282,"context_line":"        session.flush()"},{"line_number":283,"context_line":"        LOG.info(\"Total expired tokens removed: %d\", total_removed)"}],"source_content_type":"text/x-python","patch_set":7,"id":"baada198_eb82e050","line":283,"in_reply_to":"baada198_75022d3f","updated":"2014-07-18 21:55:10.000000000","message":"Ah good point. Thanks!","commit_id":"cdfbcb2f659c0c2905b0199587995604ceb47ed6"},{"author":{"_account_id":10540,"name":"Clayton O\u0027Neill","email":"clayton.oneill@charter.com","username":"coneill"},"change_message_id":"603fa2ebfdfc872b6c24f8159786df93465f9b37","unresolved":false,"context_lines":[{"line_number":280,"context_line":"            total_removed +\u003d row_count"},{"line_number":281,"context_line":""},{"line_number":282,"context_line":"        session.flush()"},{"line_number":283,"context_line":"        LOG.info(\"Total expired tokens removed: %d\", total_removed)"}],"source_content_type":"text/x-python","patch_set":7,"id":"baada198_c747cc1b","line":283,"in_reply_to":"baada198_75722d7d","updated":"2014-07-18 23:58:44.000000000","message":"Done","commit_id":"cdfbcb2f659c0c2905b0199587995604ceb47ed6"},{"author":{"_account_id":6488,"name":"Clint Byrum","email":"clint@fewbar.com","username":"clint-fewbar"},"change_message_id":"02ce6b0d9dd26781625dbd47380a05a6e489c949","unresolved":false,"context_lines":[{"line_number":280,"context_line":"            total_removed +\u003d row_count"},{"line_number":281,"context_line":""},{"line_number":282,"context_line":"        session.flush()"},{"line_number":283,"context_line":"        LOG.info(\"Total expired tokens removed: %d\", total_removed)"}],"source_content_type":"text/x-python","patch_set":7,"id":"baada198_75022d3f","line":283,"in_reply_to":"baada198_75722d7d","updated":"2014-07-18 21:14:20.000000000","message":"INFO is appropriate when it is intended to record a unit of work:\n\nhttps://wiki.openstack.org/wiki/LoggingStandards#INFO","commit_id":"cdfbcb2f659c0c2905b0199587995604ceb47ed6"},{"author":{"_account_id":6486,"name":"Brant Knudson","email":"blk@acm.org","username":"blk-u"},"change_message_id":"58f07801e930841eb1fbc650cb76630943494dfb","unresolved":false,"context_lines":[{"line_number":18,"context_line":"from keystone.common import sql"},{"line_number":19,"context_line":"from keystone import config"},{"line_number":20,"context_line":"from keystone import exception"},{"line_number":21,"context_line":"from keystone.openstack.common.gettextutils import _LI  # noqa"},{"line_number":22,"context_line":"from keystone.openstack.common import log"},{"line_number":23,"context_line":"from keystone.openstack.common import timeutils"},{"line_number":24,"context_line":"from keystone import token"}],"source_content_type":"text/x-python","patch_set":8,"id":"baada198_ca92739e","line":21,"updated":"2014-07-19 01:39:30.000000000","message":"remove the # noqa. Also, use keystone.i18n rather than gettextutils.","commit_id":"1a8a567567ffc9ce85bf31c0a5155915e8275c52"},{"author":{"_account_id":10540,"name":"Clayton O\u0027Neill","email":"clayton.oneill@charter.com","username":"coneill"},"change_message_id":"ecb19a0a848bb034c3281cf65817f6e1ce21f1c7","unresolved":false,"context_lines":[{"line_number":18,"context_line":"from keystone.common import sql"},{"line_number":19,"context_line":"from keystone import config"},{"line_number":20,"context_line":"from keystone import exception"},{"line_number":21,"context_line":"from keystone.openstack.common.gettextutils import _LI  # noqa"},{"line_number":22,"context_line":"from keystone.openstack.common import log"},{"line_number":23,"context_line":"from keystone.openstack.common import timeutils"},{"line_number":24,"context_line":"from keystone import token"}],"source_content_type":"text/x-python","patch_set":8,"id":"baada198_9c651747","line":21,"in_reply_to":"baada198_ca92739e","updated":"2014-07-20 16:07:02.000000000","message":"Apparently keystone.i18n didn\u0027t exist when I initially submitted this, so I was a bit confused.  I\u0027ve rebased and changed this to use keystone.i18n.","commit_id":"1a8a567567ffc9ce85bf31c0a5155915e8275c52"},{"author":{"_account_id":6486,"name":"Brant Knudson","email":"blk@acm.org","username":"blk-u"},"change_message_id":"58f07801e930841eb1fbc650cb76630943494dfb","unresolved":false,"context_lines":[{"line_number":195,"context_line":"            tokens.append(record)"},{"line_number":196,"context_line":"        return tokens"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"    def _expiry_range_batched(self, session, upper_bound_func, batch_size):"},{"line_number":199,"context_line":"        \"\"\"Delete expired tokens in batches"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"        Return the timestamp of the next token that is `batch_size` rows from"}],"source_content_type":"text/x-python","patch_set":8,"id":"baada198_6a3de78d","line":198,"updated":"2014-07-19 01:39:30.000000000","message":"I think it would make more sense if this were a free function rather than a member. \n\nIt\u0027s unnecessarily complicated to have a function that has access to the members of the class in addition to its arguments used a function pointer... if you\u0027re going to use functools.partial then you can pass in the arguments, and if you\u0027ve got a class method then set instance data. Since you wouldn\u0027t expect to be accessing instance data here make it a free function.","commit_id":"1a8a567567ffc9ce85bf31c0a5155915e8275c52"},{"author":{"_account_id":10540,"name":"Clayton O\u0027Neill","email":"clayton.oneill@charter.com","username":"coneill"},"change_message_id":"ecb19a0a848bb034c3281cf65817f6e1ce21f1c7","unresolved":false,"context_lines":[{"line_number":195,"context_line":"            tokens.append(record)"},{"line_number":196,"context_line":"        return tokens"},{"line_number":197,"context_line":""},{"line_number":198,"context_line":"    def _expiry_range_batched(self, session, upper_bound_func, batch_size):"},{"line_number":199,"context_line":"        \"\"\"Delete expired tokens in batches"},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"        Return the timestamp of the next token that is `batch_size` rows from"}],"source_content_type":"text/x-python","patch_set":8,"id":"baada198_7c95a3e2","line":198,"in_reply_to":"baada198_6a3de78d","updated":"2014-07-20 16:07:02.000000000","message":"Done","commit_id":"1a8a567567ffc9ce85bf31c0a5155915e8275c52"},{"author":{"_account_id":6486,"name":"Brant Knudson","email":"blk@acm.org","username":"blk-u"},"change_message_id":"58f07801e930841eb1fbc650cb76630943494dfb","unresolved":false,"context_lines":[{"line_number":226,"context_line":"            yield next_expiration"},{"line_number":227,"context_line":"        yield upper_bound_func()"},{"line_number":228,"context_line":""},{"line_number":229,"context_line":"    def _expiry_range_all(self, session, upper_bound_func):"},{"line_number":230,"context_line":"        \"\"\"Expires all tokens in one pass.\"\"\""},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"        yield upper_bound_func()"}],"source_content_type":"text/x-python","patch_set":8,"id":"baada198_4a8d8330","line":229,"updated":"2014-07-19 01:39:30.000000000","message":"same argument here. Make it a free function.","commit_id":"1a8a567567ffc9ce85bf31c0a5155915e8275c52"},{"author":{"_account_id":10540,"name":"Clayton O\u0027Neill","email":"clayton.oneill@charter.com","username":"coneill"},"change_message_id":"ecb19a0a848bb034c3281cf65817f6e1ce21f1c7","unresolved":false,"context_lines":[{"line_number":226,"context_line":"            yield next_expiration"},{"line_number":227,"context_line":"        yield upper_bound_func()"},{"line_number":228,"context_line":""},{"line_number":229,"context_line":"    def _expiry_range_all(self, session, upper_bound_func):"},{"line_number":230,"context_line":"        \"\"\"Expires all tokens in one pass.\"\"\""},{"line_number":231,"context_line":""},{"line_number":232,"context_line":"        yield upper_bound_func()"}],"source_content_type":"text/x-python","patch_set":8,"id":"baada198_5cf53f4f","line":229,"in_reply_to":"baada198_4a8d8330","updated":"2014-07-20 16:07:02.000000000","message":"Done","commit_id":"1a8a567567ffc9ce85bf31c0a5155915e8275c52"},{"author":{"_account_id":6486,"name":"Brant Knudson","email":"blk@acm.org","username":"blk-u"},"change_message_id":"58f07801e930841eb1fbc650cb76630943494dfb","unresolved":false,"context_lines":[{"line_number":234,"context_line":"    def _expiry_range_strategy(self, dialect):"},{"line_number":235,"context_line":"        \"\"\"Choose a token range expiration strategy"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"        Based on the DB dialect, select a expiry range function that is"},{"line_number":238,"context_line":"        appropriate."},{"line_number":239,"context_line":"        \"\"\""},{"line_number":240,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"baada198_6aefa71d","line":237,"updated":"2014-07-19 01:39:30.000000000","message":"suggest change \"function\" to \"callable\"","commit_id":"1a8a567567ffc9ce85bf31c0a5155915e8275c52"},{"author":{"_account_id":10540,"name":"Clayton O\u0027Neill","email":"clayton.oneill@charter.com","username":"coneill"},"change_message_id":"ecb19a0a848bb034c3281cf65817f6e1ce21f1c7","unresolved":false,"context_lines":[{"line_number":234,"context_line":"    def _expiry_range_strategy(self, dialect):"},{"line_number":235,"context_line":"        \"\"\"Choose a token range expiration strategy"},{"line_number":236,"context_line":""},{"line_number":237,"context_line":"        Based on the DB dialect, select a expiry range function that is"},{"line_number":238,"context_line":"        appropriate."},{"line_number":239,"context_line":"        \"\"\""},{"line_number":240,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"baada198_5c70df07","line":237,"in_reply_to":"baada198_6aefa71d","updated":"2014-07-20 16:07:02.000000000","message":"Done","commit_id":"1a8a567567ffc9ce85bf31c0a5155915e8275c52"},{"author":{"_account_id":6486,"name":"Brant Knudson","email":"blk@acm.org","username":"blk-u"},"change_message_id":"58f07801e930841eb1fbc650cb76630943494dfb","unresolved":false,"context_lines":[{"line_number":240,"context_line":""},{"line_number":241,"context_line":"        expiry_range_func \u003d self._expiry_range_all"},{"line_number":242,"context_line":"        if dialect \u003d\u003d \u0027ibm_db_sa\u0027:"},{"line_number":243,"context_line":"            # This functionality is limited to DB2, because"},{"line_number":244,"context_line":"            # it is necessary to prevent the tranaction log"},{"line_number":245,"context_line":"            # from filling up, whereas at least some of the"},{"line_number":246,"context_line":"            # other supported databases do not support update"}],"source_content_type":"text/x-python","patch_set":8,"id":"baada198_eae2d72e","line":243,"updated":"2014-07-19 01:39:30.000000000","message":"batching isn\u0027t limited to DB2 anymore so this comment should be cleaned up.","commit_id":"1a8a567567ffc9ce85bf31c0a5155915e8275c52"},{"author":{"_account_id":10540,"name":"Clayton O\u0027Neill","email":"clayton.oneill@charter.com","username":"coneill"},"change_message_id":"ecb19a0a848bb034c3281cf65817f6e1ce21f1c7","unresolved":false,"context_lines":[{"line_number":240,"context_line":""},{"line_number":241,"context_line":"        expiry_range_func \u003d self._expiry_range_all"},{"line_number":242,"context_line":"        if dialect \u003d\u003d \u0027ibm_db_sa\u0027:"},{"line_number":243,"context_line":"            # This functionality is limited to DB2, because"},{"line_number":244,"context_line":"            # it is necessary to prevent the tranaction log"},{"line_number":245,"context_line":"            # from filling up, whereas at least some of the"},{"line_number":246,"context_line":"            # other supported databases do not support update"}],"source_content_type":"text/x-python","patch_set":8,"id":"baada198_3c732b17","line":243,"in_reply_to":"baada198_eae2d72e","updated":"2014-07-20 16:07:02.000000000","message":"Done","commit_id":"1a8a567567ffc9ce85bf31c0a5155915e8275c52"},{"author":{"_account_id":6486,"name":"Brant Knudson","email":"blk@acm.org","username":"blk-u"},"change_message_id":"58f07801e930841eb1fbc650cb76630943494dfb","unresolved":false,"context_lines":[{"line_number":248,"context_line":"            # to require the use of such queries when deleting"},{"line_number":249,"context_line":"            # large numbers of records at once."},{"line_number":250,"context_line":"            expiry_range_func \u003d functools.partial(self._expiry_range_batched,"},{"line_number":251,"context_line":"                                                  batch_size\u003d100)"},{"line_number":252,"context_line":"            # Limit of 100 is known to not fill a transaction log"},{"line_number":253,"context_line":"            # of default maximum size while not significantly"},{"line_number":254,"context_line":"            # impacting the performance of large token purges on"}],"source_content_type":"text/x-python","patch_set":8,"id":"baada198_cad99384","line":251,"updated":"2014-07-19 01:39:30.000000000","message":"I think it would be more obvious to just return from these legs.","commit_id":"1a8a567567ffc9ce85bf31c0a5155915e8275c52"},{"author":{"_account_id":10540,"name":"Clayton O\u0027Neill","email":"clayton.oneill@charter.com","username":"coneill"},"change_message_id":"ecb19a0a848bb034c3281cf65817f6e1ce21f1c7","unresolved":false,"context_lines":[{"line_number":248,"context_line":"            # to require the use of such queries when deleting"},{"line_number":249,"context_line":"            # large numbers of records at once."},{"line_number":250,"context_line":"            expiry_range_func \u003d functools.partial(self._expiry_range_batched,"},{"line_number":251,"context_line":"                                                  batch_size\u003d100)"},{"line_number":252,"context_line":"            # Limit of 100 is known to not fill a transaction log"},{"line_number":253,"context_line":"            # of default maximum size while not significantly"},{"line_number":254,"context_line":"            # impacting the performance of large token purges on"}],"source_content_type":"text/x-python","patch_set":8,"id":"baada198_fc6c5338","line":251,"in_reply_to":"baada198_cad99384","updated":"2014-07-20 16:07:02.000000000","message":"Agreed, Done","commit_id":"1a8a567567ffc9ce85bf31c0a5155915e8275c52"},{"author":{"_account_id":6486,"name":"Brant Knudson","email":"blk@acm.org","username":"blk-u"},"change_message_id":"58f07801e930841eb1fbc650cb76630943494dfb","unresolved":false,"context_lines":[{"line_number":273,"context_line":"        expiry_range_func \u003d self._expiry_range_strategy(dialect)"},{"line_number":274,"context_line":"        query \u003d session.query(TokenModel.expires)"},{"line_number":275,"context_line":"        total_removed \u003d 0"},{"line_number":276,"context_line":"        for expiry_time in expiry_range_func(session, timeutils.utcnow):"},{"line_number":277,"context_line":"            delete_query \u003d query.filter(TokenModel.expires \u003c"},{"line_number":278,"context_line":"                                        expiry_time)"},{"line_number":279,"context_line":"            row_count \u003d delete_query.delete(synchronize_session\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":8,"id":"baada198_4a0ae3d2","line":276,"updated":"2014-07-19 01:39:30.000000000","message":"suggest give a name to \"timeutils.utcnow\" so it\u0027s obvious what it\u0027s used for --\n\n upper_bound_func\u003dtimeutils.utcnow\n\nalso, since the function will always the same don\u0027t pass it in. Use timeutils.utcnow in the function rather than passing it in.","commit_id":"1a8a567567ffc9ce85bf31c0a5155915e8275c52"},{"author":{"_account_id":10540,"name":"Clayton O\u0027Neill","email":"clayton.oneill@charter.com","username":"coneill"},"change_message_id":"ecb19a0a848bb034c3281cf65817f6e1ce21f1c7","unresolved":false,"context_lines":[{"line_number":273,"context_line":"        expiry_range_func \u003d self._expiry_range_strategy(dialect)"},{"line_number":274,"context_line":"        query \u003d session.query(TokenModel.expires)"},{"line_number":275,"context_line":"        total_removed \u003d 0"},{"line_number":276,"context_line":"        for expiry_time in expiry_range_func(session, timeutils.utcnow):"},{"line_number":277,"context_line":"            delete_query \u003d query.filter(TokenModel.expires \u003c"},{"line_number":278,"context_line":"                                        expiry_time)"},{"line_number":279,"context_line":"            row_count \u003d delete_query.delete(synchronize_session\u003dFalse)"}],"source_content_type":"text/x-python","patch_set":8,"id":"baada198_bc7f3bde","line":276,"in_reply_to":"baada198_4a0ae3d2","updated":"2014-07-20 16:07:02.000000000","message":"Done","commit_id":"1a8a567567ffc9ce85bf31c0a5155915e8275c52"},{"author":{"_account_id":6486,"name":"Brant Knudson","email":"blk@acm.org","username":"blk-u"},"change_message_id":"58f07801e930841eb1fbc650cb76630943494dfb","unresolved":false,"context_lines":[{"line_number":277,"context_line":"            delete_query \u003d query.filter(TokenModel.expires \u003c"},{"line_number":278,"context_line":"                                        expiry_time)"},{"line_number":279,"context_line":"            row_count \u003d delete_query.delete(synchronize_session\u003dFalse)"},{"line_number":280,"context_line":"            LOG.info(_LI(\u0027Removed %d expired tokens\u0027), row_count)"},{"line_number":281,"context_line":"            total_removed +\u003d row_count"},{"line_number":282,"context_line":""},{"line_number":283,"context_line":"        session.flush()"}],"source_content_type":"text/x-python","patch_set":8,"id":"baada198_8a831b02","line":280,"updated":"2014-07-19 01:39:30.000000000","message":"this could wind up being a lot of output printed really quickly (especially for db2 since we\u0027re only doing 100 rows at a time). I\u0027d suggest debug. Also, I think we\u0027d be better off switching to printing total_removed rather than row_count since it\u0027ll just be the batch size (or should be close to it).","commit_id":"1a8a567567ffc9ce85bf31c0a5155915e8275c52"},{"author":{"_account_id":10540,"name":"Clayton O\u0027Neill","email":"clayton.oneill@charter.com","username":"coneill"},"change_message_id":"ecb19a0a848bb034c3281cf65817f6e1ce21f1c7","unresolved":false,"context_lines":[{"line_number":277,"context_line":"            delete_query \u003d query.filter(TokenModel.expires \u003c"},{"line_number":278,"context_line":"                                        expiry_time)"},{"line_number":279,"context_line":"            row_count \u003d delete_query.delete(synchronize_session\u003dFalse)"},{"line_number":280,"context_line":"            LOG.info(_LI(\u0027Removed %d expired tokens\u0027), row_count)"},{"line_number":281,"context_line":"            total_removed +\u003d row_count"},{"line_number":282,"context_line":""},{"line_number":283,"context_line":"        session.flush()"}],"source_content_type":"text/x-python","patch_set":8,"id":"baada198_9c7ef7e2","line":280,"in_reply_to":"baada198_8a831b02","updated":"2014-07-20 16:07:02.000000000","message":"Done","commit_id":"1a8a567567ffc9ce85bf31c0a5155915e8275c52"},{"author":{"_account_id":5707,"name":"Henry Nash","email":"henryn@linux.vnet.ibm.com","username":"henry-nash"},"change_message_id":"229bb29ac8da8277d26173003ab01e0422a29051","unresolved":false,"context_lines":[{"line_number":45,"context_line":""},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"def _expiry_range_batched(session, upper_bound_func, batch_size):"},{"line_number":48,"context_line":"    \"\"\"Delete expired tokes in batches"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    Return the timestamp of the next token that is `batch_size` rows from"},{"line_number":51,"context_line":"    being the oldest expired token."}],"source_content_type":"text/x-python","patch_set":10,"id":"baada198_b054bac6","line":48,"updated":"2014-07-20 21:18:44.000000000","message":"I don\u0027t think this function actually deletes them, does it?","commit_id":"06a652a5f5f41e4700ac647798b7d863c509d1f5"},{"author":{"_account_id":10540,"name":"Clayton O\u0027Neill","email":"clayton.oneill@charter.com","username":"coneill"},"change_message_id":"c055a5e09af708ea05ae9ebd4b296c64a793db53","unresolved":false,"context_lines":[{"line_number":45,"context_line":""},{"line_number":46,"context_line":""},{"line_number":47,"context_line":"def _expiry_range_batched(session, upper_bound_func, batch_size):"},{"line_number":48,"context_line":"    \"\"\"Delete expired tokes in batches"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    Return the timestamp of the next token that is `batch_size` rows from"},{"line_number":51,"context_line":"    being the oldest expired token."}],"source_content_type":"text/x-python","patch_set":10,"id":"baada198_8b9a2b12","line":48,"in_reply_to":"baada198_b054bac6","updated":"2014-07-21 01:15:26.000000000","message":"You\u0027re right, this comment needs some work.  I\u0027ll have a new change set in a bit.","commit_id":"06a652a5f5f41e4700ac647798b7d863c509d1f5"},{"author":{"_account_id":5707,"name":"Henry Nash","email":"henryn@linux.vnet.ibm.com","username":"henry-nash"},"change_message_id":"229bb29ac8da8277d26173003ab01e0422a29051","unresolved":false,"context_lines":[{"line_number":51,"context_line":"    being the oldest expired token."},{"line_number":52,"context_line":"    \"\"\""},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    # If we\u0027re deleting in batches then we will get the next row that"},{"line_number":55,"context_line":"    # is `batch_size` away from the oldest token.  We\u0027ll then delete"},{"line_number":56,"context_line":"    # all the tokens up to the time on that token.  This may delete"},{"line_number":57,"context_line":"    # slightly more tokens than the batch_size, but that should be ok"}],"source_content_type":"text/x-python","patch_set":10,"id":"baada198_10f646d7","line":54,"updated":"2014-07-20 21:18:44.000000000","message":"Again, if this function doesn\u0027t actually delete them, then this wording feels a bit odd.","commit_id":"06a652a5f5f41e4700ac647798b7d863c509d1f5"},{"author":{"_account_id":10540,"name":"Clayton O\u0027Neill","email":"clayton.oneill@charter.com","username":"coneill"},"change_message_id":"c055a5e09af708ea05ae9ebd4b296c64a793db53","unresolved":false,"context_lines":[{"line_number":51,"context_line":"    being the oldest expired token."},{"line_number":52,"context_line":"    \"\"\""},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"    # If we\u0027re deleting in batches then we will get the next row that"},{"line_number":55,"context_line":"    # is `batch_size` away from the oldest token.  We\u0027ll then delete"},{"line_number":56,"context_line":"    # all the tokens up to the time on that token.  This may delete"},{"line_number":57,"context_line":"    # slightly more tokens than the batch_size, but that should be ok"}],"source_content_type":"text/x-python","patch_set":10,"id":"baada198_ab9b6f15","line":54,"in_reply_to":"baada198_10f646d7","updated":"2014-07-21 01:15:26.000000000","message":"Done","commit_id":"06a652a5f5f41e4700ac647798b7d863c509d1f5"}]}