)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"5a772f498f25613e3ad8e6e368715dbda446d923","unresolved":true,"context_lines":[{"line_number":67,"context_line":"like it\u0027s not necessary since the DB already allows null values, but it"},{"line_number":68,"context_line":"seems more correct to make sure that\u0027s always the case."},{"line_number":69,"context_line":""},{"line_number":70,"context_line":"This patch doesn\u0027t close but #1961102 because the os-brick patch is"},{"line_number":71,"context_line":"needed for that."},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"Related-Bug: #1961102"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"0ef59359_5a02aa50","line":70,"range":{"start_line":70,"start_character":25,"end_line":70,"end_character":28},"updated":"2022-07-26 22:43:25.000000000","message":"bug","commit_id":"ef741228d8a0509f21fa560619d9504ebb940bac"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ac90967c1a5ed3dcd64a6fb9f99fd9f0be83c649","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"12ff8005_dc008795","updated":"2022-07-27 07:05:18.000000000","message":"Few queries and nits inline but can be resolved later. LGTM.","commit_id":"ef741228d8a0509f21fa560619d9504ebb940bac"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"5a772f498f25613e3ad8e6e368715dbda446d923","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"63abb35c_a101867c","updated":"2022-07-26 22:43:25.000000000","message":"Some issues noted inline, but nothing that can\u0027t be done in a followup before the Zed release.  Code and tests look fine.","commit_id":"ef741228d8a0509f21fa560619d9504ebb940bac"}],"api-ref/source/v3/parameters.yaml":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"5a772f498f25613e3ad8e6e368715dbda446d923","unresolved":true,"context_lines":[{"line_number":2687,"context_line":"shared_targets_tristate:"},{"line_number":2688,"context_line":"  description: |"},{"line_number":2689,"context_line":"    An indicator whether the host connecting the volume should lock for the"},{"line_number":2690,"context_line":"    whole attach/detach process or not. ``true`` means only is iSCSI initiator"},{"line_number":2691,"context_line":"    running on host doesn\u0027t support manual scans, ``false`` means never use"},{"line_number":2692,"context_line":"    locks, and ``null`` means to always use locks. Look at os-brick\u0027s"},{"line_number":2693,"context_line":"    ``guard_connection`` context manager.  Default\u003dTrue."}],"source_content_type":"text/x-yaml","patch_set":4,"id":"fb198ea9_053397bf","line":2690,"range":{"start_line":2690,"start_character":55,"end_line":2690,"end_character":62},"updated":"2022-07-26 22:43:25.000000000","message":"s/only is/only lock when the/","commit_id":"ef741228d8a0509f21fa560619d9504ebb940bac"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"5a772f498f25613e3ad8e6e368715dbda446d923","unresolved":true,"context_lines":[{"line_number":2689,"context_line":"    An indicator whether the host connecting the volume should lock for the"},{"line_number":2690,"context_line":"    whole attach/detach process or not. ``true`` means only is iSCSI initiator"},{"line_number":2691,"context_line":"    running on host doesn\u0027t support manual scans, ``false`` means never use"},{"line_number":2692,"context_line":"    locks, and ``null`` means to always use locks. Look at os-brick\u0027s"},{"line_number":2693,"context_line":"    ``guard_connection`` context manager.  Default\u003dTrue."},{"line_number":2694,"context_line":"  in: body"},{"line_number":2695,"context_line":"  required: true"},{"line_number":2696,"context_line":"  type: boolean"}],"source_content_type":"text/x-yaml","patch_set":4,"id":"cdbdb393_1920d1c3","line":2693,"range":{"start_line":2692,"start_character":51,"end_line":2693,"end_character":41},"updated":"2022-07-26 22:43:25.000000000","message":"maybe something like:\n\nUseful when you are using os-brick to manually connect a volume; see os-brick\u0027s ``guard_connection`` context manager.  The value is set by cinder based on the storage backend\u0027s connection properties.\n\n(I\u0027m not sure that it makes sense to mention the default value here, because it\u0027s not something the user has any control over.  Further, if they are using mv\u003c3.48, they won\u0027t see shared_targets in the response, but its absence doesn\u0027t mean that its value is True, so I think that mentioning the default is misleading.)","commit_id":"ef741228d8a0509f21fa560619d9504ebb940bac"}],"api-ref/source/v3/samples/volumes/v3.69/volume-create-response.json":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"5a772f498f25613e3ad8e6e368715dbda446d923","unresolved":true,"context_lines":[{"line_number":33,"context_line":"        \"group_id\": null,"},{"line_number":34,"context_line":"        \"provider_id\": null,"},{"line_number":35,"context_line":"        \"service_uuid\": null,"},{"line_number":36,"context_line":"        \"shared_targets\": null,"},{"line_number":37,"context_line":"        \"cluster_name\": null,"},{"line_number":38,"context_line":"        \"volume_type_id\": \"5fed9d7c-401d-46e2-8e80-f30c70cb7e1d\","},{"line_number":39,"context_line":"        \"consumes_quota\": true"}],"source_content_type":"application/json","patch_set":4,"id":"238ca3a3_384a7d62","line":36,"range":{"start_line":36,"start_character":26,"end_line":36,"end_character":30},"updated":"2022-07-26 22:43:25.000000000","message":"nit: any reason not to show this with the default value (true)?","commit_id":"ef741228d8a0509f21fa560619d9504ebb940bac"}],"cinder/db/migrations/versions/c92a3e68beed_make_shared_targets_nullable.py":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ac90967c1a5ed3dcd64a6fb9f99fd9f0be83c649","unresolved":true,"context_lines":[{"line_number":23,"context_line":""},{"line_number":24,"context_line":"# revision identifiers, used by Alembic."},{"line_number":25,"context_line":"revision \u003d \u0027c92a3e68beed\u0027"},{"line_number":26,"context_line":"down_revision \u003d \u0027921e1a36b076\u0027"},{"line_number":27,"context_line":"branch_labels \u003d None"},{"line_number":28,"context_line":"depends_on \u003d None"},{"line_number":29,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"1e56406f_225bb456","line":26,"range":{"start_line":26,"start_character":0,"end_line":26,"end_character":30},"updated":"2022-07-27 07:05:18.000000000","message":"alembic does support downgrading migrations but is it useful for cinder?","commit_id":"ef741228d8a0509f21fa560619d9504ebb940bac"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ac90967c1a5ed3dcd64a6fb9f99fd9f0be83c649","unresolved":true,"context_lines":[{"line_number":35,"context_line":"    table \u003d sa.Table(\u0027volumes\u0027, sa.MetaData(), autoload_with\u003dconnection)"},{"line_number":36,"context_line":"    existing_type \u003d table.c.shared_targets.type"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    # SQLite doesn\u0027t support altering tables, so we use a workaround"},{"line_number":39,"context_line":"    if connection.engine.name \u003d\u003d \u0027sqlite\u0027:"},{"line_number":40,"context_line":"        with op.batch_alter_table(\u0027volumes\u0027) as batch_op:"},{"line_number":41,"context_line":"            batch_op.alter_column(\u0027shared_targets\u0027,"}],"source_content_type":"text/x-python","patch_set":4,"id":"151e0d6e_d52f55e1","line":38,"range":{"start_line":38,"start_character":58,"end_line":38,"end_character":68},"updated":"2022-07-27 07:05:18.000000000","message":"is it a workaround or alternative way of doing it?","commit_id":"ef741228d8a0509f21fa560619d9504ebb940bac"}],"cinder/db/sqlalchemy/models.py":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"5a772f498f25613e3ad8e6e368715dbda446d923","unresolved":true,"context_lines":[{"line_number":405,"context_line":"    # True \u003d\u003e Do locking when iSCSI initiator doesn\u0027t support manual scan"},{"line_number":406,"context_line":"    # False \u003d\u003e Never do locking"},{"line_number":407,"context_line":"    # None \u003d\u003e Forced locking regardless of the iSCSI initiator"},{"line_number":408,"context_line":"    # make an FK of service?"},{"line_number":409,"context_line":"    shared_targets \u003d sa.Column(sa.Boolean, nullable\u003dTrue, default\u003dTrue)"},{"line_number":410,"context_line":""},{"line_number":411,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"620cfcdc_12690dce","line":408,"range":{"start_line":408,"start_character":0,"end_line":408,"end_character":28},"updated":"2022-07-26 22:43:25.000000000","message":"nit: I think this existing comment is supposed to apply to lines 399-404; should probably either delete it or move it to before line 399","commit_id":"ef741228d8a0509f21fa560619d9504ebb940bac"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ac90967c1a5ed3dcd64a6fb9f99fd9f0be83c649","unresolved":true,"context_lines":[{"line_number":405,"context_line":"    # True \u003d\u003e Do locking when iSCSI initiator doesn\u0027t support manual scan"},{"line_number":406,"context_line":"    # False \u003d\u003e Never do locking"},{"line_number":407,"context_line":"    # None \u003d\u003e Forced locking regardless of the iSCSI initiator"},{"line_number":408,"context_line":"    # make an FK of service?"},{"line_number":409,"context_line":"    shared_targets \u003d sa.Column(sa.Boolean, nullable\u003dTrue, default\u003dTrue)"},{"line_number":410,"context_line":""},{"line_number":411,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"4a361bf5_092c821f","line":408,"range":{"start_line":408,"start_character":0,"end_line":408,"end_character":28},"in_reply_to":"620cfcdc_12690dce","updated":"2022-07-27 07:05:18.000000000","message":"I agree but since this comment is not added by this patch (it already exists in our codebase) so we can do it in a separate patch","commit_id":"ef741228d8a0509f21fa560619d9504ebb940bac"}],"cinder/tests/unit/db/test_migrations.py":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ac90967c1a5ed3dcd64a6fb9f99fd9f0be83c649","unresolved":true,"context_lines":[{"line_number":135,"context_line":"    # Migrations can take a long time, particularly on underpowered CI nodes."},{"line_number":136,"context_line":"    # Give them some breathing room."},{"line_number":137,"context_line":"    TIMEOUT_SCALING_FACTOR \u003d 4"},{"line_number":138,"context_line":"    BOOL_TYPE \u003d sqlalchemy.types.BOOLEAN"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"    def setUp(self):"},{"line_number":141,"context_line":"        super().setUp()"}],"source_content_type":"text/x-python","patch_set":4,"id":"6b686957_723ba864","line":138,"range":{"start_line":138,"start_character":4,"end_line":138,"end_character":40},"updated":"2022-07-27 07:05:18.000000000","message":"not sure if this is supposed to be used somewhere explicitly or has an implicit effect on tests?","commit_id":"ef741228d8a0509f21fa560619d9504ebb940bac"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ac90967c1a5ed3dcd64a6fb9f99fd9f0be83c649","unresolved":true,"context_lines":[{"line_number":198,"context_line":"            self.config,"},{"line_number":199,"context_line":"        ).get_current_head()"},{"line_number":200,"context_line":"        self.assertEqual(head, migration.db_version())"},{"line_number":201,"context_line":""},{"line_number":202,"context_line":"    def _pre_upgrade_c92a3e68beed(self, connection):"},{"line_number":203,"context_line":"        \"\"\"Test shared_targets is nullable.\"\"\""},{"line_number":204,"context_line":"        table \u003d db_utils.get_table(connection, \u0027volumes\u0027)"},{"line_number":205,"context_line":"        self._previous_type \u003d type(table.c.shared_targets.type)"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    def _check_c92a3e68beed(self, connection):"},{"line_number":208,"context_line":"        \"\"\"Test shared_targets is nullable.\"\"\""},{"line_number":209,"context_line":"        table \u003d db_utils.get_table(connection, \u0027volumes\u0027)"},{"line_number":210,"context_line":"        self.assertIn(\u0027shared_targets\u0027, table.c)"},{"line_number":211,"context_line":"        # Type hasn\u0027t changed"},{"line_number":212,"context_line":"        self.assertIsInstance(table.c.shared_targets.type, self._previous_type)"},{"line_number":213,"context_line":"        # But it\u0027s nullable"},{"line_number":214,"context_line":"        self.assertTrue(table.c.shared_targets.nullable)"},{"line_number":215,"context_line":""},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"class TestMigrationsWalkSQLite("},{"line_number":218,"context_line":"    MigrationsWalk,"}],"source_content_type":"text/x-python","patch_set":4,"id":"e842b405_10f9fdfb","line":215,"range":{"start_line":201,"start_character":0,"end_line":215,"end_character":0},"updated":"2022-07-27 07:05:18.000000000","message":"these are private methods with no \"test_\" prefix, are they being executed?","commit_id":"ef741228d8a0509f21fa560619d9504ebb940bac"}],"cinder/volume/manager.py":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ac90967c1a5ed3dcd64a6fb9f99fd9f0be83c649","unresolved":true,"context_lines":[{"line_number":856,"context_line":"        return volume.id"},{"line_number":857,"context_line":""},{"line_number":858,"context_line":"    def _driver_shares_targets(self):"},{"line_number":859,"context_line":"        \"\"\"Report if driver shares targets and needs locking on connecing side."},{"line_number":860,"context_line":""},{"line_number":861,"context_line":"        This is currently only relevant for iSCSI and for NVMe-oF."},{"line_number":862,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"d9243b5c_e45bf125","line":859,"range":{"start_line":859,"start_character":64,"end_line":859,"end_character":73},"updated":"2022-07-27 07:05:18.000000000","message":"nit: connecting","commit_id":"ef741228d8a0509f21fa560619d9504ebb940bac"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"ac90967c1a5ed3dcd64a6fb9f99fd9f0be83c649","unresolved":true,"context_lines":[{"line_number":869,"context_line":"        from the subsystem, and they are automatically removed when unmapped"},{"line_number":870,"context_line":"        via an asynchronous message from the storage system."},{"line_number":871,"context_line":""},{"line_number":872,"context_line":"        By exposing the shared_targets characteristic in the volume nova (and"},{"line_number":873,"context_line":"        other consumers) can use os-brick\u0027s specific context manager to prevent"},{"line_number":874,"context_line":"        these race conditions."},{"line_number":875,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"6f03f5ba_03d35eb2","line":872,"range":{"start_line":872,"start_character":67,"end_line":872,"end_character":68},"updated":"2022-07-27 07:05:18.000000000","message":"nit:comma here","commit_id":"ef741228d8a0509f21fa560619d9504ebb940bac"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"5a772f498f25613e3ad8e6e368715dbda446d923","unresolved":true,"context_lines":[{"line_number":881,"context_line":"                  ISCSI_SUPPORTS_MANUAL_SCAN.  Used by NVMe-oF."},{"line_number":882,"context_line":""},{"line_number":883,"context_line":"        Drivers can return in their capabilities shared_targets set to ``None``"},{"line_number":884,"context_line":"        to force locking on the host side regarless of the protocol"},{"line_number":885,"context_line":"        \"\"\""},{"line_number":886,"context_line":"        capabilities \u003d self.driver.capabilities"},{"line_number":887,"context_line":"        # We default to True to be on the safe side."}],"source_content_type":"text/x-python","patch_set":4,"id":"d55b2b5f_a3284c44","line":884,"range":{"start_line":884,"start_character":42,"end_line":884,"end_character":51},"updated":"2022-07-26 22:43:25.000000000","message":"regardless","commit_id":"ef741228d8a0509f21fa560619d9504ebb940bac"}]}