)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0c556e342fa88ad28bfdb81fdfb5a871a2a71756","unresolved":true,"context_lines":[{"line_number":19,"context_line":"This may not be the desired behavior for CLI tooling. Furthermore,"},{"line_number":20,"context_line":"quarantining a database attempts to move it to a relative location"},{"line_number":21,"context_line":"assuming the canonical layout, which may be nonsense for database files"},{"line_number":22,"context_line":"that have been relocated for forensics. This change adds an optional"},{"line_number":23,"context_line":"quarantine_ok flag to the database broker constructor and modifies the"},{"line_number":24,"context_line":"CLI tools to opt out of quarantine."},{"line_number":25,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"ff3498ed_8f51e74d","line":22,"updated":"2026-09-18 18:45:27.000000000","message":"\u003e which may be nonsense for database files that have been relocated for forensics.\n\nPERFECT - we definately need this change.","commit_id":"cccc414d0df8c856609d8a02f677a98663ce6afa"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0c556e342fa88ad28bfdb81fdfb5a871a2a71756","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"ba28819f_537414cf","updated":"2026-09-18 18:45:27.000000000","message":"1006319: sq? Cover container info quarantine opt-out | https://review.opendev.org/c/openstack/swift/+/1006319\n\nI think the doc-string should change\nI think we should cover the container case\n\nit\u0027s a good patch tho - nice commit message - helpful 🙏","commit_id":"cccc414d0df8c856609d8a02f677a98663ce6afa"}],"swift/cli/info.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0c556e342fa88ad28bfdb81fdfb5a871a2a71756","unresolved":true,"context_lines":[{"line_number":486,"context_line":"        datadir \u003d ABDATADIR"},{"line_number":487,"context_line":"    else:"},{"line_number":488,"context_line":"        broker \u003d ContainerBroker(db_file, stale_reads_ok\u003dstale_reads_ok,"},{"line_number":489,"context_line":"                                 quarantine_ok\u003dFalse)"},{"line_number":490,"context_line":"        datadir \u003d CBDATADIR"},{"line_number":491,"context_line":"    try:"},{"line_number":492,"context_line":"        info \u003d broker.get_info()"}],"source_content_type":"text/x-python","patch_set":2,"id":"8e39a518_9697ea75","line":489,"updated":"2026-09-18 18:45:27.000000000","message":"probably better as `broker_class \u003d AccountBroker` so you can apply `quarnatine_ok\u003dFalse` consistently","commit_id":"cccc414d0df8c856609d8a02f677a98663ce6afa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0c556e342fa88ad28bfdb81fdfb5a871a2a71756","unresolved":true,"context_lines":[{"line_number":506,"context_line":"    except sqlite3.OperationalError as err:"},{"line_number":507,"context_line":"        if \u0027no such table\u0027 in str(err):"},{"line_number":508,"context_line":"            print(\"Does not appear to be a DB of type \\\"%s\\\": %s\""},{"line_number":509,"context_line":"                  % (db_type, db_file))"},{"line_number":510,"context_line":"            raise InfoSystemExit()"},{"line_number":511,"context_line":"        raise"},{"line_number":512,"context_line":"    except sqlite3.DatabaseError as err:"}],"source_content_type":"text/x-python","patch_set":2,"id":"b2dc9c89_369729c9","line":509,"updated":"2026-09-18 18:45:27.000000000","message":"this sort of looks like the error we were thiking about making quarantine-able?\n\ni.e. \"if auditor tries to check on account db and finds a container db the container db should be quarnatined\"\n\nobvs that behavior is specific to the container - an operator may have just fat fingered a path and printing an error and NOT quarantineing is definately the right behavior.","commit_id":"cccc414d0df8c856609d8a02f677a98663ce6afa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0c556e342fa88ad28bfdb81fdfb5a871a2a71756","unresolved":true,"context_lines":[{"line_number":522,"context_line":"                    incoming, include_timestamp\u003dTrue))"},{"line_number":523,"context_line":"        except sqlite3.DatabaseError as err:"},{"line_number":524,"context_line":"            print_db_error(err)"},{"line_number":525,"context_line":"            raise InfoSystemExit()"},{"line_number":526,"context_line":"    try:"},{"line_number":527,"context_line":"        ring \u003d Ring(swift_dir, ring_name\u003ddb_type)"},{"line_number":528,"context_line":"    except Exception:"}],"source_content_type":"text/x-python","patch_set":2,"id":"6ac9f0cc_0fcbf623","line":525,"updated":"2026-09-18 18:45:27.000000000","message":"the smell for me is the similarity to the exception block above....\n\nthis may be the right diff *tactically* but presumably a better design for the `print_info` method would be to sequester ALL the database queries into an extracted helper so we can handle `BrokerError` consistently:\n\n```\ntry:\n    datdir, metadata, syncs \u003d _query_the_broker(db_type, db_file, stale_ok)\nexcept BrokerError:\n    print_db_error(err)\n    raise InfoSystemExit()\nprint_db_info_metadata()\nprint_both_syncs(syncs)\n...\n```","commit_id":"cccc414d0df8c856609d8a02f677a98663ce6afa"}],"swift/common/db.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0c556e342fa88ad28bfdb81fdfb5a871a2a71756","unresolved":true,"context_lines":[{"line_number":333,"context_line":"        :param quarantine_ok: if False then quarantine() never moves the"},{"line_number":334,"context_line":"            database; the caller still learns the reason via the raised"},{"line_number":335,"context_line":"            sqlite3.DatabaseError, but the file is left in place for"},{"line_number":336,"context_line":"            inspection. Read-only tooling should set this False."},{"line_number":337,"context_line":"        \"\"\""},{"line_number":338,"context_line":"        self.conn \u003d None"},{"line_number":339,"context_line":"        self._db_file \u003d db_file"}],"source_content_type":"text/x-python","patch_set":2,"id":"dd045532_3196aef5","line":336,"updated":"2026-09-18 18:45:27.000000000","message":"instead of Read-only I prefer \"diagnostic\" - or even \"operator\" - basically \"anyone not a consistency engine daemon\"\n\nthe combination of `quarantine_ok\u003dFalse \u0026 (skip_commits\u003dFalse || stale_reads_ok\u003dFalse)` as valid suggests to me that it \"doesn\u0027t modify the database\" is not the REAL constraint for when this should be False\n\n... in fact I think it should DEFAULT to False; although; we might need a `\u003dNone` deprecation period 😭","commit_id":"cccc414d0df8c856609d8a02f677a98663ce6afa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0c556e342fa88ad28bfdb81fdfb5a871a2a71756","unresolved":true,"context_lines":[{"line_number":523,"context_line":"        (malformed or corrupted database). If not, the original exception will"},{"line_number":524,"context_line":"        be reraised. If so, the database will be quarantined -- unless the"},{"line_number":525,"context_line":"        broker was constructed with ``quarantine_ok`` False, in which case it"},{"line_number":526,"context_line":"        is left in place -- and a new sqlite3.DatabaseError will be raised."},{"line_number":527,"context_line":"        \"\"\""},{"line_number":528,"context_line":"        if \u0027database disk image is malformed\u0027 in str(err):"},{"line_number":529,"context_line":"            exc_hint \u003d \u0027malformed database\u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"1877281d_22c88c28","line":526,"updated":"2026-09-18 18:45:27.000000000","message":"I think the design I\u0027d ultimately like is Brokers to raise BrokerErrors - when/if those seem quarantinable the caller should have say:\n\n```\nwith quarantineable(db_file):\n   self._get_a_broker_and_do_all_my_db_stuff(db_file)\n```\n\neveryone else should just blow up with `BrokerError(\u0027useful diagnostic info\u0027)`\n\n... that said I don\u0027t know *exactly* what the implementation of `quarantieable` should do other than:\n\n```\ntry:\n    yield\nexcept BrokerError as e:\n    if ??????:\n        quarnatine(db_file)\n    else:\n        logging.exception???\n        re-raise?\n```\n\nso, since i\u0027m just dreaming - this is a fine start tho 💪","commit_id":"cccc414d0df8c856609d8a02f677a98663ce6afa"}],"test/unit/cli/test_info.py":[{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0c556e342fa88ad28bfdb81fdfb5a871a2a71756","unresolved":true,"context_lines":[{"line_number":793,"context_line":"            finally:"},{"line_number":794,"context_line":"                os.chdir(orig_cwd)"},{"line_number":795,"context_line":"        if exp_raised:"},{"line_number":796,"context_line":"            exp_out \u003d \u0027Does not appear to be a DB of type \"account\":\u0027 \\"},{"line_number":797,"context_line":"                \u0027 ./d49d0ecbb53be1fcc49624f2f7c7ccae.db\u0027"},{"line_number":798,"context_line":"            self.assertEqual(out.getvalue().strip(), exp_out)"},{"line_number":799,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"774a2ef0_83385c6e","line":796,"updated":"2026-09-18 18:45:27.000000000","message":"even if this *technically* covers the regression below - I prefer:\n\n    test_print_info_wrong_db_type_message_still_works\n\nover\n\n    test_print_info\n\nPerhaps better as:\n\n    test_print_info_malformed_wrong_type_precedence\n    \n... with a comment like:\n\n    # given both we think wrong type is MOST useful starting point to debug","commit_id":"cccc414d0df8c856609d8a02f677a98663ce6afa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0c556e342fa88ad28bfdb81fdfb5a871a2a71756","unresolved":true,"context_lines":[{"line_number":841,"context_line":"                                     \u0027HTTP_X_TIMESTAMP\u0027: \u00270\u0027})"},{"line_number":842,"context_line":"        resp \u003d req.get_response(controller)"},{"line_number":843,"context_line":"        self.assertEqual(resp.status_int, 201)"},{"line_number":844,"context_line":"        db_file \u003d os.path.join(self.testdir, \u0027sda1\u0027, \u0027containers\u0027,"},{"line_number":845,"context_line":"                               \u00271\u0027, \u0027cae\u0027,"},{"line_number":846,"context_line":"                               \u0027d49d0ecbb53be1fcc49624f2f7c7ccae\u0027,"},{"line_number":847,"context_line":"                               \u0027d49d0ecbb53be1fcc49624f2f7c7ccae.db\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"8aefbafa_f713fe5e","line":844,"updated":"2026-09-18 18:45:27.000000000","message":"AFAICT this is the only new test that uses a container and doesn\u0027t exercise the new code\n\nconsider:\n\n```\ndiff --git a/swift/cli/info.py b/swift/cli/info.py\nindex 17276d70c..f7fbf6e9b 100644\n--- a/swift/cli/info.py\n+++ b/swift/cli/info.py\n@@ -485,8 +485,7 @@ def print_info(db_type, db_file, swift_dir\u003d\u0027/etc/swift\u0027, stale_reads_ok\u003dFalse,\n                                quarantine_ok\u003dFalse)\n         datadir \u003d ABDATADIR\n     else:\n-        broker \u003d ContainerBroker(db_file, stale_reads_ok\u003dstale_reads_ok,\n-                                 quarantine_ok\u003dFalse)\n+        broker \u003d ContainerBroker(db_file, stale_reads_ok\u003dstale_reads_ok)\n         datadir \u003d CBDATADIR\n     try:\n         info \u003d broker.get_info()\n```\n\n```\npytest test/unit/cli/test_info.py\n...\n55 passed in 0.82s\n```","commit_id":"cccc414d0df8c856609d8a02f677a98663ce6afa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0c556e342fa88ad28bfdb81fdfb5a871a2a71756","unresolved":true,"context_lines":[{"line_number":858,"context_line":"        # not a sqlite3.OperationalError one, so it reaches the same except"},{"line_number":859,"context_line":"        # clause as a real quarantine reason. It is not necessarily damage"},{"line_number":860,"context_line":"        # (e.g. a permission error opening an otherwise healthy file), so"},{"line_number":861,"context_line":"        # the message must not call it \"damaged\"."},{"line_number":862,"context_line":"        controller \u003d AccountController("},{"line_number":863,"context_line":"            {\u0027devices\u0027: self.testdir, \u0027mount_check\u0027: \u0027false\u0027})"},{"line_number":864,"context_line":"        req \u003d Request.blank(\u0027/sda1/1/acct\u0027, environ\u003d{\u0027REQUEST_METHOD\u0027: \u0027PUT\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"a6ce449d_8dfd64cd","line":861,"updated":"2026-09-18 18:45:27.000000000","message":"seems like somebodies agent was getting a bit too strong with with their language!","commit_id":"cccc414d0df8c856609d8a02f677a98663ce6afa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0c556e342fa88ad28bfdb81fdfb5a871a2a71756","unresolved":true,"context_lines":[{"line_number":878,"context_line":"            print_info(\u0027account\u0027, db_file, swift_dir\u003dself.testdir)"},{"line_number":879,"context_line":"        self.assertEqual("},{"line_number":880,"context_line":"            out.getvalue().strip(),"},{"line_number":881,"context_line":"            \u0027Database error: %s\\n\u0027"},{"line_number":882,"context_line":"            \u0027This command does not quarantine; the database has been \u0027"},{"line_number":883,"context_line":"            \u0027left in place.\u0027 % conn_err)"},{"line_number":884,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"e3121555_44ab684d","line":881,"updated":"2026-09-18 18:45:27.000000000","message":"I bet this assertion would be stronger if it was a literal string.","commit_id":"cccc414d0df8c856609d8a02f677a98663ce6afa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0c556e342fa88ad28bfdb81fdfb5a871a2a71756","unresolved":true,"context_lines":[{"line_number":882,"context_line":"            \u0027This command does not quarantine; the database has been \u0027"},{"line_number":883,"context_line":"            \u0027left in place.\u0027 % conn_err)"},{"line_number":884,"context_line":""},{"line_number":885,"context_line":"    def test_print_info_sync_reports_damage_instead_of_tracebacking(self):"},{"line_number":886,"context_line":"        # broker.get_syncs(), reached only when sync\u003dTrue, goes through the"},{"line_number":887,"context_line":"        # same self.get() -\u003e possibly_quarantine() path as get_info() and"},{"line_number":888,"context_line":"        # friends but is called after the main try/except block; it needs"}],"source_content_type":"text/x-python","patch_set":2,"id":"853dc174_0aa3ebe8","line":885,"updated":"2026-09-18 18:45:27.000000000","message":"I love verbing 🤣","commit_id":"cccc414d0df8c856609d8a02f677a98663ce6afa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0c556e342fa88ad28bfdb81fdfb5a871a2a71756","unresolved":true,"context_lines":[{"line_number":886,"context_line":"        # broker.get_syncs(), reached only when sync\u003dTrue, goes through the"},{"line_number":887,"context_line":"        # same self.get() -\u003e possibly_quarantine() path as get_info() and"},{"line_number":888,"context_line":"        # friends but is called after the main try/except block; it needs"},{"line_number":889,"context_line":"        # its own damage handling."},{"line_number":890,"context_line":"        controller \u003d AccountController("},{"line_number":891,"context_line":"            {\u0027devices\u0027: self.testdir, \u0027mount_check\u0027: \u0027false\u0027})"},{"line_number":892,"context_line":"        req \u003d Request.blank(\u0027/sda1/1/acct\u0027, environ\u003d{\u0027REQUEST_METHOD\u0027: \u0027PUT\u0027,"}],"source_content_type":"text/x-python","patch_set":2,"id":"3afcea0e_ca60fdd8","line":889,"updated":"2026-09-18 18:45:27.000000000","message":"DAMAGE","commit_id":"cccc414d0df8c856609d8a02f677a98663ce6afa"},{"author":{"_account_id":1179,"name":"Clay Gerrard","email":"clay.gerrard@gmail.com","username":"clay-gerrard"},"change_message_id":"0c556e342fa88ad28bfdb81fdfb5a871a2a71756","unresolved":true,"context_lines":[{"line_number":906,"context_line":"            print_info(\u0027account\u0027, db_file, swift_dir\u003dself.testdir, sync\u003dTrue)"},{"line_number":907,"context_line":"        self.assertEqual("},{"line_number":908,"context_line":"            out.getvalue().strip().splitlines()[-2:],"},{"line_number":909,"context_line":"            [\u0027Database error: malformed database\u0027,"},{"line_number":910,"context_line":"             \u0027This command does not quarantine; the database has been \u0027"},{"line_number":911,"context_line":"             \u0027left in place.\u0027])"},{"line_number":912,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"adc20e51_23288084","line":909,"updated":"2026-09-18 18:45:27.000000000","message":"this is helpful context.","commit_id":"cccc414d0df8c856609d8a02f677a98663ce6afa"}]}