)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":14228,"name":"Clif Houck","email":"me@clifhouck.com","username":"clif_h"},"change_message_id":"a76ce491fd67f5f874996e0671c4489e2ab7da21","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"3acad91d_4f3ecc43","updated":"2026-04-06 20:31:51.000000000","message":"A couple of comments. At a minimum needs some unit tests before it can go further.","commit_id":"eb8fe2ee84d04c5b03fde35326bdb2dfd42a4646"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"b2defe6471d9238008785cbd46d4e34d25fc1507","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"1c2f16b8_313085f3","updated":"2026-04-06 20:45:06.000000000","message":"Greetings! Yes, I think we\u0027re going to need a unit test to minimally demonstrate the bug and verify that these change are indeed fixing the issue.","commit_id":"eb8fe2ee84d04c5b03fde35326bdb2dfd42a4646"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"963b1aad347149ff047035820c88d241a726aa7a","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"48a3f7d5_c76d518e","updated":"2026-05-26 20:33:29.000000000","message":"Feel free to re-add the ironic-week-prio hashtag once review feedback has been taken into consideration.","commit_id":"db7c2d3331c43e55cc25baa01d5a36b17613bc45"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"14faaab2212a5cf37b200590d7274e6f5b8ce182","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"2956aaf3_59134f4b","updated":"2026-04-28 17:39:39.000000000","message":"This was part of the GR-OSS group review. There\u0027s a general unease at the level of complexity this change requires even though we were unable to immediately identify a simpler approach.\n\nIn the future, we should be extremely hesitant to add filters to our API which do not directly translate to the DB to avoid these patterns.","commit_id":"db7c2d3331c43e55cc25baa01d5a36b17613bc45"}],"ironic/api/controllers/v1/node.py":[{"author":{"_account_id":14228,"name":"Clif Houck","email":"me@clifhouck.com","username":"clif_h"},"change_message_id":"a76ce491fd67f5f874996e0671c4489e2ab7da21","unresolved":true,"context_lines":[{"line_number":2534,"context_line":""},{"line_number":2535,"context_line":"        # Special filtering on results based on conductor field"},{"line_number":2536,"context_line":"        if conductor:"},{"line_number":2537,"context_line":"            nodes \u003d self._filter_by_conductor(nodes, conductor)"},{"line_number":2538,"context_line":""},{"line_number":2539,"context_line":"        parameters \u003d {\u0027sort_key\u0027: sort_key, \u0027sort_dir\u0027: sort_dir}"},{"line_number":2540,"context_line":"        if associated:"}],"source_content_type":"text/x-python","patch_set":2,"id":"f24a3648_5daf1c6c","side":"PARENT","line":2537,"updated":"2026-04-06 20:31:51.000000000","message":"So it seems like we\u0027re substituting one filter for another and from what I can tell the semantics of each filter are slightly different. `_filter_by_conductor`, if I\u0027m reading it correctly, ultimately uses the hash-ring to deterministically pick the conductor to which a node belongs, while `conductor_affinity` is more like \"which conductor last operated on the node\". These *may* be equivalent in practice, but that is not clear to me.\n\nI\u0027m not sure if this is the right approach or not. I\u0027ll defer to someone more knowledgeable, but this substitution makes me nervous.","commit_id":"eda12b9caf56ce6c454b0ff75a8e50acfe5f5f58"},{"author":{"_account_id":5890,"name":"Doug Goldstein","email":"cardoe@cardoe.com","username":"cardoe"},"change_message_id":"6ae1bcd417d0826aeb4600156f803d48b3d5f567","unresolved":true,"context_lines":[{"line_number":2534,"context_line":""},{"line_number":2535,"context_line":"        # Special filtering on results based on conductor field"},{"line_number":2536,"context_line":"        if conductor:"},{"line_number":2537,"context_line":"            nodes \u003d self._filter_by_conductor(nodes, conductor)"},{"line_number":2538,"context_line":""},{"line_number":2539,"context_line":"        parameters \u003d {\u0027sort_key\u0027: sort_key, \u0027sort_dir\u0027: sort_dir}"},{"line_number":2540,"context_line":"        if associated:"}],"source_content_type":"text/x-python","patch_set":2,"id":"93e7c258_9ad7b898","side":"PARENT","line":2537,"in_reply_to":"95d82889_f45904f8","updated":"2026-04-07 20:15:50.000000000","message":"I think we need to backport this.","commit_id":"eda12b9caf56ce6c454b0ff75a8e50acfe5f5f58"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"b2defe6471d9238008785cbd46d4e34d25fc1507","unresolved":true,"context_lines":[{"line_number":2534,"context_line":""},{"line_number":2535,"context_line":"        # Special filtering on results based on conductor field"},{"line_number":2536,"context_line":"        if conductor:"},{"line_number":2537,"context_line":"            nodes \u003d self._filter_by_conductor(nodes, conductor)"},{"line_number":2538,"context_line":""},{"line_number":2539,"context_line":"        parameters \u003d {\u0027sort_key\u0027: sort_key, \u0027sort_dir\u0027: sort_dir}"},{"line_number":2540,"context_line":"        if associated:"}],"source_content_type":"text/x-python","patch_set":2,"id":"95d82889_f45904f8","side":"PARENT","line":2537,"in_reply_to":"f24a3648_5daf1c6c","updated":"2026-04-06 20:45:06.000000000","message":"Indeed! Technically affinity should just be the last node but this is the hash ring result. I guess you *can* end up with a mismatch based upon state which might be what is being encountered?!","commit_id":"eda12b9caf56ce6c454b0ff75a8e50acfe5f5f58"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"14faaab2212a5cf37b200590d7274e6f5b8ce182","unresolved":true,"context_lines":[{"line_number":2504,"context_line":"            \u0027sharded\u0027: sharded,"},{"line_number":2505,"context_line":"            \u0027include_children\u0027: include_children,"},{"line_number":2506,"context_line":"            \u0027parent_node\u0027: parent_node"},{"line_number":2507,"context_line":"            }"},{"line_number":2508,"context_line":""},{"line_number":2509,"context_line":"        filters \u003d {}"},{"line_number":2510,"context_line":"        for key, value in possible_filters.items():"}],"source_content_type":"text/x-python","patch_set":4,"id":"e2ba72e9_f15c983a","line":2507,"updated":"2026-04-28 17:39:39.000000000","message":"This removes a trailing comma, it\u0027s an automatic reformatting that makes the code worse. Please revert these lines.","commit_id":"db7c2d3331c43e55cc25baa01d5a36b17613bc45"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"14faaab2212a5cf37b200590d7274e6f5b8ce182","unresolved":true,"context_lines":[{"line_number":2537,"context_line":"        else:"},{"line_number":2538,"context_line":"            nodes \u003d []"},{"line_number":2539,"context_line":"            current_marker_obj \u003d marker_obj"},{"line_number":2540,"context_line":"            db_batch_size \u003d CONF.api.max_limit"},{"line_number":2541,"context_line":""},{"line_number":2542,"context_line":"            while len(nodes) \u003c limit:"},{"line_number":2543,"context_line":"                chunk \u003d objects.Node.list(api.request.context,"}],"source_content_type":"text/x-python","patch_set":4,"id":"c6cdf263_59c42372","line":2540,"updated":"2026-04-28 17:39:39.000000000","message":"this should be min(CONF.api.max_limit, limit)","commit_id":"db7c2d3331c43e55cc25baa01d5a36b17613bc45"},{"author":{"_account_id":10342,"name":"Jay Faulkner","display_name":"JayF","email":"jay@jvf.cc","username":"JayF","status":"youtube.com/@oss-gr / podcast.gr-oss.io"},"change_message_id":"14faaab2212a5cf37b200590d7274e6f5b8ce182","unresolved":true,"context_lines":[{"line_number":2540,"context_line":"            db_batch_size \u003d CONF.api.max_limit"},{"line_number":2541,"context_line":""},{"line_number":2542,"context_line":"            while len(nodes) \u003c limit:"},{"line_number":2543,"context_line":"                chunk \u003d objects.Node.list(api.request.context,"},{"line_number":2544,"context_line":"                                          limit\u003ddb_batch_size,"},{"line_number":2545,"context_line":"                                          marker\u003dcurrent_marker_obj,"},{"line_number":2546,"context_line":"                                          sort_key\u003dsort_key,"}],"source_content_type":"text/x-python","patch_set":4,"id":"37f641d9_18d63566","line":2543,"updated":"2026-04-28 17:39:39.000000000","message":"I\u0027m curious how this impacts query performance. I don\u0027t think this is a super common query, so I\u0027m not that worried about it, but we\u0027re absolutely changing performance semantics here and ideally we\u0027d run a quick benchmark to validate it works well enough.","commit_id":"db7c2d3331c43e55cc25baa01d5a36b17613bc45"}],"ironic/db/sqlalchemy/api.py":[{"author":{"_account_id":14228,"name":"Clif Houck","email":"me@clifhouck.com","username":"clif_h"},"change_message_id":"a76ce491fd67f5f874996e0671c4489e2ab7da21","unresolved":true,"context_lines":[{"line_number":480,"context_line":"    _NODE_QUERY_FIELDS \u003d {\u0027console_enabled\u0027, \u0027maintenance\u0027, \u0027retired\u0027,"},{"line_number":481,"context_line":"                          \u0027driver\u0027, \u0027resource_class\u0027, \u0027provision_state\u0027,"},{"line_number":482,"context_line":"                          \u0027uuid\u0027, \u0027id\u0027, \u0027fault\u0027, \u0027conductor_group\u0027,"},{"line_number":483,"context_line":"                          \u0027conductor_affinity\u0027, \u0027owner\u0027, \u0027lessee\u0027,"},{"line_number":484,"context_line":"                          \u0027instance_uuid\u0027, \u0027instance_name\u0027}"},{"line_number":485,"context_line":"    _NODE_IN_QUERY_FIELDS \u003d {\u0027%s_in\u0027 % field: field"},{"line_number":486,"context_line":"                             for field in (\u0027uuid\u0027, \u0027provision_state\u0027, \u0027shard\u0027)}"}],"source_content_type":"text/x-python","patch_set":2,"id":"555b74ff_28c8a4ab","line":483,"updated":"2026-04-06 20:31:51.000000000","message":"According to the NOTE above, you should also update `get_nodeinfo_list`\u0027s docstring in ironic/db/api.py","commit_id":"eb8fe2ee84d04c5b03fde35326bdb2dfd42a4646"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"b2defe6471d9238008785cbd46d4e34d25fc1507","unresolved":true,"context_lines":[{"line_number":480,"context_line":"    _NODE_QUERY_FIELDS \u003d {\u0027console_enabled\u0027, \u0027maintenance\u0027, \u0027retired\u0027,"},{"line_number":481,"context_line":"                          \u0027driver\u0027, \u0027resource_class\u0027, \u0027provision_state\u0027,"},{"line_number":482,"context_line":"                          \u0027uuid\u0027, \u0027id\u0027, \u0027fault\u0027, \u0027conductor_group\u0027,"},{"line_number":483,"context_line":"                          \u0027conductor_affinity\u0027, \u0027owner\u0027, \u0027lessee\u0027,"},{"line_number":484,"context_line":"                          \u0027instance_uuid\u0027, \u0027instance_name\u0027}"},{"line_number":485,"context_line":"    _NODE_IN_QUERY_FIELDS \u003d {\u0027%s_in\u0027 % field: field"},{"line_number":486,"context_line":"                             for field in (\u0027uuid\u0027, \u0027provision_state\u0027, \u0027shard\u0027)}"}],"source_content_type":"text/x-python","patch_set":2,"id":"e5444577_423c91f0","line":483,"in_reply_to":"555b74ff_28c8a4ab","updated":"2026-04-06 20:45:06.000000000","message":"we also, generally speaking, try to only add to the tail end on lists unless we need to inject earlier in the list.","commit_id":"eb8fe2ee84d04c5b03fde35326bdb2dfd42a4646"}]}