)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"2249141e5674ffd9bca25afc83698e669c012c9a","unresolved":true,"context_lines":[{"line_number":4,"context_line":"Commit:     rajivmucheli \u003crajiv.mucheli@gmail.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2024-12-24 19:11:43 +0530"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"ccdddPreserve URL Filters in pagination logic"},{"line_number":8,"context_line":"PEP8 fixes"},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"fixes https://bugs.launchpad.net/barbican/+bug/2064583"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"2eacf135_9486dbf1","line":7,"range":{"start_line":7,"start_character":0,"end_line":7,"end_character":5},"updated":"2025-01-07 15:44:31.000000000","message":"What are these ?","commit_id":"6fa093ab26484c00b743383daa38bc535e33c947"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"2249141e5674ffd9bca25afc83698e669c012c9a","unresolved":true,"context_lines":[{"line_number":5,"context_line":"CommitDate: 2024-12-24 19:11:43 +0530"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"ccdddPreserve URL Filters in pagination logic"},{"line_number":8,"context_line":"PEP8 fixes"},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"fixes https://bugs.launchpad.net/barbican/+bug/2064583"},{"line_number":11,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"04fc3197_868c44e3","line":8,"range":{"start_line":8,"start_character":0,"end_line":8,"end_character":10},"updated":"2025-01-07 15:44:31.000000000","message":"I run pep8 against the latest code but no error is detected. If you mean you fixed pep8 errors \"caused by your change\" then this is not needed because it\u0027s always required that any pep8 error is fixed when code is changed.","commit_id":"6fa093ab26484c00b743383daa38bc535e33c947"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"2249141e5674ffd9bca25afc83698e669c012c9a","unresolved":true,"context_lines":[{"line_number":7,"context_line":"ccdddPreserve URL Filters in pagination logic"},{"line_number":8,"context_line":"PEP8 fixes"},{"line_number":9,"context_line":""},{"line_number":10,"context_line":"fixes https://bugs.launchpad.net/barbican/+bug/2064583"},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Change-Id: I0baa85dca00352148f9d5be58283c9157474704d"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"63613b35_4ec23746","line":11,"range":{"start_line":10,"start_character":0,"end_line":11,"end_character":1},"updated":"2025-01-07 15:44:31.000000000","message":"Closes-Bug: #2064583\n\nis the correct format.","commit_id":"6fa093ab26484c00b743383daa38bc535e33c947"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":35125,"name":"Mauricio Harley","email":"mharley@redhat.com","username":"mharley-rh"},"change_message_id":"6b2b6484308fac32c5c707ba70d263a7c453e3e0","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":7,"id":"d6e357dc_fd0e7e2c","updated":"2026-03-27 15:30:52.000000000","message":"Thanks for working on this bug fix - preserving query filters in pagination links is a valid improvement.\n\nHowever, I have some concerns with the current approach:\n\n1. Fix should be in hrefs.add_nav_hrefs(): This shared function handles pagination for all endpoints (secrets, orders, containers). Replacing it with inline logic only in the secrets controller leaves the bug unfixed for other endpoints and introduces inconsistency. The fix should be in the shared function.\n\nUnrelated changes: Several cosmetic reformattings, added comments, and removed comments are unrelated to the pagination bug and should be in a separate patch.\n\nPlease respond to the 7 unresolved comments from the previous review.\n\nSee also my inline comments for details.","commit_id":"fdba0f58347de2c484ff8c19bfff0a91a0f6b337"}],"barbican/api/controllers/secrets.py":[{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"2249141e5674ffd9bca25afc83698e669c012c9a","unresolved":true,"context_lines":[{"line_number":383,"context_line":""},{"line_number":384,"context_line":"        # Ensure offset and limit are properly cast to integers"},{"line_number":385,"context_line":"        offset \u003d kw.get(\u0027offset\u0027, 0)"},{"line_number":386,"context_line":"        if isinstance(offset, list):"},{"line_number":387,"context_line":"            offset \u003d offset[0]"},{"line_number":388,"context_line":"        try:"},{"line_number":389,"context_line":"            offset \u003d int(offset)"}],"source_content_type":"text/x-python","patch_set":3,"id":"98020828_23c103fc","line":386,"range":{"start_line":386,"start_character":30,"end_line":386,"end_character":34},"updated":"2025-01-07 15:44:31.000000000","message":"Do you mind explaining actual example \"valid\" input which results in a list value ?","commit_id":"6fa093ab26484c00b743383daa38bc535e33c947"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"2249141e5674ffd9bca25afc83698e669c012c9a","unresolved":true,"context_lines":[{"line_number":388,"context_line":"        try:"},{"line_number":389,"context_line":"            offset \u003d int(offset)"},{"line_number":390,"context_line":"        except ValueError:"},{"line_number":391,"context_line":"            offset \u003d 0  # Default to 0 if invalid"},{"line_number":392,"context_line":""},{"line_number":393,"context_line":"        limit \u003d kw.get(\u0027limit\u0027, 10)"},{"line_number":394,"context_line":"        if isinstance(limit, list):"}],"source_content_type":"text/x-python","patch_set":3,"id":"85598208_f426f138","line":391,"range":{"start_line":391,"start_character":12,"end_line":391,"end_character":49},"updated":"2025-01-07 15:44:31.000000000","message":"An explicit BadRequest error should be returned in case an invalid input is given, instead of ignoring it.","commit_id":"6fa093ab26484c00b743383daa38bc535e33c947"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"2249141e5674ffd9bca25afc83698e669c012c9a","unresolved":true,"context_lines":[{"line_number":391,"context_line":"            offset \u003d 0  # Default to 0 if invalid"},{"line_number":392,"context_line":""},{"line_number":393,"context_line":"        limit \u003d kw.get(\u0027limit\u0027, 10)"},{"line_number":394,"context_line":"        if isinstance(limit, list):"},{"line_number":395,"context_line":"            limit \u003d limit[0]"},{"line_number":396,"context_line":"        try:"},{"line_number":397,"context_line":"            limit \u003d int(limit)"}],"source_content_type":"text/x-python","patch_set":3,"id":"7b167fa1_c3bc2dc4","line":394,"range":{"start_line":394,"start_character":29,"end_line":394,"end_character":33},"updated":"2025-01-07 15:44:31.000000000","message":"Do you mind explaining actual example \"valid\" input which results in a list value ?","commit_id":"6fa093ab26484c00b743383daa38bc535e33c947"},{"author":{"_account_id":9816,"name":"Takashi Kajinami","email":"kajinamit@oss.nttdata.com","username":"kajinamit"},"change_message_id":"2249141e5674ffd9bca25afc83698e669c012c9a","unresolved":true,"context_lines":[{"line_number":396,"context_line":"        try:"},{"line_number":397,"context_line":"            limit \u003d int(limit)"},{"line_number":398,"context_line":"        except ValueError:"},{"line_number":399,"context_line":"            limit \u003d 10"},{"line_number":400,"context_line":""},{"line_number":401,"context_line":"        bits \u003d kw.get(\u0027bits\u0027, 0)"},{"line_number":402,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"498ee531_87b104ec","line":399,"range":{"start_line":399,"start_character":12,"end_line":399,"end_character":22},"updated":"2025-01-07 15:44:31.000000000","message":"An explicit BadRequest error should be returned in case an invalid input is given, instead of ignoring it.","commit_id":"6fa093ab26484c00b743383daa38bc535e33c947"},{"author":{"_account_id":35125,"name":"Mauricio Harley","email":"mharley@redhat.com","username":"mharley-rh"},"change_message_id":"6b2b6484308fac32c5c707ba70d263a7c453e3e0","unresolved":true,"context_lines":[{"line_number":365,"context_line":"    @controllers.enforce_rbac(\u0027secrets:get\u0027)"},{"line_number":366,"context_line":"    def on_get(self, external_project_id, **kw):"},{"line_number":367,"context_line":"        no_consumers \u003d versions.is_supported(pecan.request, max_version\u003d\u00271.0\u0027)"},{"line_number":368,"context_line":"        # NOTE(xek): consumers are being introduced in 1.1"},{"line_number":369,"context_line":""},{"line_number":370,"context_line":"        def secret_fields(field):"},{"line_number":371,"context_line":"            resp \u003d putil.mime_types.augment_fields_with_content_types(field)"}],"source_content_type":"text/x-python","patch_set":7,"id":"5a606de7_1e40df1a","side":"PARENT","line":368,"range":{"start_line":368,"start_character":0,"end_line":368,"end_character":58},"updated":"2026-03-27 15:30:52.000000000","message":"This comment removal is unrelated to the pagination fix. Please keep unrelated changes out of this patch.","commit_id":"f8a331a40eb21e6c8f37e07794d57aa98b120af9"},{"author":{"_account_id":35125,"name":"Mauricio Harley","email":"mharley@redhat.com","username":"mharley-rh"},"change_message_id":"6b2b6484308fac32c5c707ba70d263a7c453e3e0","unresolved":true,"context_lines":[{"line_number":384,"context_line":"        try:"},{"line_number":385,"context_line":"            bits \u003d int(bits)"},{"line_number":386,"context_line":"        except ValueError:"},{"line_number":387,"context_line":"            # as per Github issue 171, if bits is invalid then"},{"line_number":388,"context_line":"            # the default should be used."},{"line_number":389,"context_line":"            bits \u003d 0"},{"line_number":390,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"948d99b0_1c671576","side":"PARENT","line":387,"range":{"start_line":387,"start_character":0,"end_line":387,"end_character":62},"updated":"2026-03-27 15:30:52.000000000","message":"This comment documented a design decision (Github issue 171). Removing it without justification loses context for future maintainers. Please keep it, or at minimum explain in the commit message why it was removed.","commit_id":"f8a331a40eb21e6c8f37e07794d57aa98b120af9"},{"author":{"_account_id":35125,"name":"Mauricio Harley","email":"mharley@redhat.com","username":"mharley-rh"},"change_message_id":"6b2b6484308fac32c5c707ba70d263a7c453e3e0","unresolved":true,"context_lines":[{"line_number":388,"context_line":"        except (ValueError, TypeError):"},{"line_number":389,"context_line":"            _bad_query_string_parameters()"},{"line_number":390,"context_line":""},{"line_number":391,"context_line":"        limit \u003d kw.get(\u0027limit\u0027, 10)"},{"line_number":392,"context_line":"        try:"},{"line_number":393,"context_line":"            limit \u003d int(limit)"},{"line_number":394,"context_line":"        except (ValueError, TypeError):"}],"source_content_type":"text/x-python","patch_set":7,"id":"5caa2e2f_1e812d00","line":391,"range":{"start_line":391,"start_character":0,"end_line":391,"end_character":35},"updated":"2026-03-27 15:30:52.000000000","message":"The previous code used kw.get(\u0027limit\u0027) (defaulting to None), letting the repository layer decide the default. Hardcoding 10 here may introduce a silent behaviour change if the repository layer has a different default. Please verify this matches and consider using the same pattern as before.","commit_id":"fdba0f58347de2c484ff8c19bfff0a91a0f6b337"},{"author":{"_account_id":35125,"name":"Mauricio Harley","email":"mharley@redhat.com","username":"mharley-rh"},"change_message_id":"6b2b6484308fac32c5c707ba70d263a7c453e3e0","unresolved":true,"context_lines":[{"line_number":448,"context_line":"            ]"},{"line_number":449,"context_line":""},{"line_number":450,"context_line":"            # Preserve query parameters for pagination"},{"line_number":451,"context_line":"            base_url \u003d pecan.request.path_url"},{"line_number":452,"context_line":"            query_params \u003d pecan.request.params.mixed()"},{"line_number":453,"context_line":""},{"line_number":454,"context_line":"            # Construct next URL"}],"source_content_type":"text/x-python","patch_set":7,"id":"f1186cf4_a21ec1ad","line":451,"range":{"start_line":451,"start_character":0,"end_line":451,"end_character":45},"updated":"2026-03-27 15:30:52.000000000","message":"This reimplements pagination URL construction inline, replacing the shared hrefs.add_nav_hrefs() function. That function is used by all paginated endpoints (secrets, orders, containers, etc.), which likely all have the same bug. The fix should be in add_nav_hrefs() itself so that all endpoints benefit. As-is, this only fixes secrets and introduces inconsistency.","commit_id":"fdba0f58347de2c484ff8c19bfff0a91a0f6b337"},{"author":{"_account_id":35125,"name":"Mauricio Harley","email":"mharley@redhat.com","username":"mharley-rh"},"change_message_id":"6b2b6484308fac32c5c707ba70d263a7c453e3e0","unresolved":true,"context_lines":[{"line_number":467,"context_line":"            else:"},{"line_number":468,"context_line":"                prev_url \u003d None"},{"line_number":469,"context_line":""},{"line_number":470,"context_line":"            secrets_resp_overall \u003d {"},{"line_number":471,"context_line":"                \u0027secrets\u0027: secrets_resp,"},{"line_number":472,"context_line":"                \u0027total\u0027: total,"},{"line_number":473,"context_line":"                \u0027next\u0027: next_url,"},{"line_number":474,"context_line":"                \u0027previous\u0027: prev_url"},{"line_number":475,"context_line":"            }"},{"line_number":476,"context_line":""},{"line_number":477,"context_line":"        LOG.info(\u0027Retrieved secret list for project: %s\u0027, external_project_id)"},{"line_number":478,"context_line":"        return secrets_resp_overall"}],"source_content_type":"text/x-python","patch_set":7,"id":"da3e4377_c12bec8d","line":475,"range":{"start_line":470,"start_character":0,"end_line":475,"end_character":13},"updated":"2026-03-27 15:30:52.000000000","message":"When there is no next/previous page, these keys will have null values in the JSON response. The current behaviour (via add_nav_hrefs) is to omit the key entirely. This is an API response shape change that could break clients checking if \u0027next\u0027 in response. If you keep this approach, the keys should be omitted when the value is None.","commit_id":"fdba0f58347de2c484ff8c19bfff0a91a0f6b337"}]}
