)]}'
{"/COMMIT_MSG":[{"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":"a4f4d75e1e14107b4ecb0f4dfdec2561ec494090","unresolved":true,"context_lines":[{"line_number":7,"context_line":"Phase 1 - SQLAlchemy 2.0 Compatability"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"One of the major changes in SQLAlchemy 2.0 is the removal"},{"line_number":10,"context_line":"of autocommit support. It turns out ironic was using this quite"},{"line_number":11,"context_line":"aggressively without even really being aware of it."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"* Moved the declaritive_base to ORM, as noted in the SQLAlchemy 2.0"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3d306436_c93313a7","line":10,"updated":"2022-09-13 22:45:34.000000000","message":"nit: Ironic","commit_id":"da66c3502f996ad15c9ee81201662cc1aa510212"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"7f92542e4e9ca277347f419b439715391f99ab02","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Phase 1 - SQLAlchemy 2.0 Compatability"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"One of the major changes in SQLAlchemy 2.0 is the removal"},{"line_number":10,"context_line":"of autocommit support. It turns out ironic was using this quite"},{"line_number":11,"context_line":"aggressively without even really being aware of it."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"* Moved the declaritive_base to ORM, as noted in the SQLAlchemy 2.0"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"7447eb3a_9afd9d1a","line":10,"in_reply_to":"3d306436_c93313a7","updated":"2022-09-27 15:17:47.000000000","message":"Done","commit_id":"da66c3502f996ad15c9ee81201662cc1aa510212"},{"author":{"_account_id":4571,"name":"Steve Baker","email":"sbaker@redhat.com","username":"steve-stevebaker"},"change_message_id":"34f52e42d117660678b696943ccdb0121be6e510","unresolved":true,"context_lines":[{"line_number":37,"context_line":""},{"line_number":38,"context_line":"* DB queries involving Traits are now loaded using SelectInLoad"},{"line_number":39,"context_line":"  as opposed to Joins. The now deprecated ORM queries were quietly"},{"line_number":40,"context_line":"  and silently de-duplicating rows nad providing consistent sets"},{"line_number":41,"context_line":"  from the resulting joined table responses, however putting much"},{"line_number":42,"context_line":"  higher CPU load on the processing of results on the client."},{"line_number":43,"context_line":"  Prior performance testing has informed us this should be a minimal"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"79fcb44c_401a7903","line":40,"range":{"start_line":40,"start_character":35,"end_line":40,"end_character":38},"updated":"2022-09-14 23:23:54.000000000","message":"nit:and","commit_id":"da66c3502f996ad15c9ee81201662cc1aa510212"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"84b8d8cb34bf7776ba76878cbd6a928b9f6de313","unresolved":false,"context_lines":[{"line_number":37,"context_line":""},{"line_number":38,"context_line":"* DB queries involving Traits are now loaded using SelectInLoad"},{"line_number":39,"context_line":"  as opposed to Joins. The now deprecated ORM queries were quietly"},{"line_number":40,"context_line":"  and silently de-duplicating rows nad providing consistent sets"},{"line_number":41,"context_line":"  from the resulting joined table responses, however putting much"},{"line_number":42,"context_line":"  higher CPU load on the processing of results on the client."},{"line_number":43,"context_line":"  Prior performance testing has informed us this should be a minimal"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"24e543c7_0d7884fe","line":40,"range":{"start_line":40,"start_character":35,"end_line":40,"end_character":38},"in_reply_to":"79fcb44c_401a7903","updated":"2022-09-23 19:35:18.000000000","message":"Done","commit_id":"da66c3502f996ad15c9ee81201662cc1aa510212"},{"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":"a4f4d75e1e14107b4ecb0f4dfdec2561ec494090","unresolved":true,"context_lines":[{"line_number":73,"context_line":"  Also, by default, everything returned is a tuple in 1.4 with the"},{"line_number":74,"context_line":"  unified interface."},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"Ultimately, I have labled the remaining places which SQLAlchemy wanrings"},{"line_number":77,"context_line":"are raised at for deprecation/removal of functionality, which needs to"},{"line_number":78,"context_line":"be addressed."},{"line_number":79,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"40b020f9_eaa536ca","line":76,"updated":"2022-09-13 22:45:34.000000000","message":"nit: warnings","commit_id":"da66c3502f996ad15c9ee81201662cc1aa510212"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"7f92542e4e9ca277347f419b439715391f99ab02","unresolved":false,"context_lines":[{"line_number":73,"context_line":"  Also, by default, everything returned is a tuple in 1.4 with the"},{"line_number":74,"context_line":"  unified interface."},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"Ultimately, I have labled the remaining places which SQLAlchemy wanrings"},{"line_number":77,"context_line":"are raised at for deprecation/removal of functionality, which needs to"},{"line_number":78,"context_line":"be addressed."},{"line_number":79,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"c8e0d45f_05ff97a5","line":76,"in_reply_to":"40b020f9_eaa536ca","updated":"2022-09-27 15:17:47.000000000","message":"Done","commit_id":"da66c3502f996ad15c9ee81201662cc1aa510212"},{"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":"835cba4dcc7ceec6de71850ab650f0bbc32fbdfc","unresolved":true,"context_lines":[{"line_number":73,"context_line":"  Also, by default, everything returned is a tuple in 1.4 with the"},{"line_number":74,"context_line":"  unified interface."},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"Ultimately, I have labled the remaining places which SQLAlchemy warnings"},{"line_number":77,"context_line":"are raised at for deprecation/removal of functionality, which needs to"},{"line_number":78,"context_line":"be addressed."},{"line_number":79,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"74df4c97_860d2d86","line":76,"updated":"2022-09-26 21:16:34.000000000","message":"nit: labelled","commit_id":"2c6f1ddb4800789d58a24f4238d65da1b511d32a"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"7f92542e4e9ca277347f419b439715391f99ab02","unresolved":false,"context_lines":[{"line_number":73,"context_line":"  Also, by default, everything returned is a tuple in 1.4 with the"},{"line_number":74,"context_line":"  unified interface."},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"Ultimately, I have labled the remaining places which SQLAlchemy warnings"},{"line_number":77,"context_line":"are raised at for deprecation/removal of functionality, which needs to"},{"line_number":78,"context_line":"be addressed."},{"line_number":79,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"c91c81a4_1a10843f","line":76,"in_reply_to":"74df4c97_860d2d86","updated":"2022-09-27 15:17:47.000000000","message":"Done","commit_id":"2c6f1ddb4800789d58a24f4238d65da1b511d32a"}],"/PATCHSET_LEVEL":[{"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":"a4f4d75e1e14107b4ecb0f4dfdec2561ec494090","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"3fb4d7fb_2bea99f0","updated":"2022-09-13 22:45:34.000000000","message":"Gonna be honest, there are some changes here I don\u0027t fully understand.\n\nIsn\u0027t there also a job we can run to check performance differences caused by this patch?","commit_id":"da66c3502f996ad15c9ee81201662cc1aa510212"},{"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":"7d711f96cc7549a5b2a62856b198e34ad3bf2e61","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":4,"id":"cfc0e348_7f70a4cf","updated":"2022-09-14 22:56:57.000000000","message":"I ran basic benchmarks on master with this patch vs master without this patch. There was not a significant enough change in execution speed to convince me this has any performance impact.","commit_id":"da66c3502f996ad15c9ee81201662cc1aa510212"},{"author":{"_account_id":4571,"name":"Steve Baker","email":"sbaker@redhat.com","username":"steve-stevebaker"},"change_message_id":"34f52e42d117660678b696943ccdb0121be6e510","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":4,"id":"8ffe5301_6416167a","updated":"2022-09-14 23:23:54.000000000","message":"This all looks reasonable so far","commit_id":"da66c3502f996ad15c9ee81201662cc1aa510212"},{"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":"8359156507a5ff49d29c115e1a186f8b03fa353a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"ade5dc89_4739404d","updated":"2022-09-26 22:18:12.000000000","message":"CI failure on phase 2 patch leads me to wonder if we have racey issues introduced by these changes; we should get to the bottom of those before it merges","commit_id":"2c6f1ddb4800789d58a24f4238d65da1b511d32a"},{"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":"835cba4dcc7ceec6de71850ab650f0bbc32fbdfc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"bd4b7c82_64e5f646","updated":"2022-09-26 21:16:34.000000000","message":"Thank you for all the good work to make this stuff work! I think it should be safe. Not re-running benchmarks as there seem to be no performance-changing changes since I ran it.","commit_id":"2c6f1ddb4800789d58a24f4238d65da1b511d32a"},{"author":{"_account_id":4571,"name":"Steve Baker","email":"sbaker@redhat.com","username":"steve-stevebaker"},"change_message_id":"fb131f5d34cf2f47e8da4c4528cd1cb2601e5616","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":8,"id":"031b0914_f58bef83","updated":"2022-09-27 02:41:29.000000000","message":"recheck potential racey test_touch_conductor_deadlock","commit_id":"2c6f1ddb4800789d58a24f4238d65da1b511d32a"},{"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":"abacd1b15f215e011323cbf97dd86f32f10af48d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"c3b5c6cc_56037e61","updated":"2022-10-06 22:41:46.000000000","message":"Julia said this likely shouldn\u0027t land before https://review.opendev.org/c/openstack/ironic/+/860198 -- so I\u0027m just W-1 until that one lands to grab attention and ensure someone won\u0027t land this one, since there\u0027s not an explicit dependency relationship between them.","commit_id":"52621b81e72feed9f29d0217286f161d3b4a0035"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"aaf6d9e22f2d9934505076fe824072973c3f6e62","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"4abba2d6_60ce4bbd","updated":"2022-09-27 19:10:15.000000000","message":"recheck jobs timed out","commit_id":"52621b81e72feed9f29d0217286f161d3b4a0035"},{"author":{"_account_id":4571,"name":"Steve Baker","email":"sbaker@redhat.com","username":"steve-stevebaker"},"change_message_id":"cdcc22efb9654280f2b069b9253b13335f833d17","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"5ad16d01_88c90842","updated":"2022-10-03 21:20:27.000000000","message":"recheck timeouts","commit_id":"52621b81e72feed9f29d0217286f161d3b4a0035"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"f5a3ce1e36b8ff97ca16021ca0380319e0ff3f05","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"60f74479_480935db","updated":"2022-10-14 18:41:46.000000000","message":"recheck no definitive indicator recorded. tempest appears to have completed or was near completion and the output was truncated by ansible. :(","commit_id":"49e085583dec81c63d19f80a9ba067e38d8043ae"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"988952d30926cd39e2791af499c3d615fed94cc8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"a30c04d5_d5d2c9ec","updated":"2022-10-14 14:27:22.000000000","message":"recheck snmp driver is mocking sleeps which needs to go away. I believe it is in the next patch already.\n\nft1.25: ironic.tests.unit.drivers.modules.test_snmp.SNMPDeviceDriverTestCase.test_power_offtesttools.testresult.real._StringException: Traceback (most recent call last):\n  File \"/usr/lib/python3.8/unittest/mock.py\", line 1325, in patched\n    return func(*newargs, **newkeywargs)\n  File \"/home/zuul/src/opendev.org/openstack/ironic/ironic/tests/unit/drivers/modules/test_snmp.py\", line 778, in test_power_off\n    mock_sleep.assert_called_once_with(1)\n  File \"/usr/lib/python3.8/unittest/mock.py\", line 209, in assert_called_once_with\n    return mock.assert_called_once_with(*args, **kwargs)\n  File \"/usr/lib/python3.8/unittest/mock.py\", line 924, in assert_called_once_with\n    raise AssertionError(msg)\nAssertionError: Expected \u0027sleep\u0027 to be called once. Called 6 times.\nCalls: [call(1), call(0), call(0), call(0), call(0), call(0)].","commit_id":"49e085583dec81c63d19f80a9ba067e38d8043ae"},{"author":{"_account_id":4571,"name":"Steve Baker","email":"sbaker@redhat.com","username":"steve-stevebaker"},"change_message_id":"67b1a7c1591518707f0f299e76e33b3f525b0979","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"3caaf2db_58c56971","updated":"2022-10-14 01:05:09.000000000","message":"recheck test_snmp power_off intermittent failure\n\nproposed fix https://review.opendev.org/c/openstack/ironic/+/861274","commit_id":"49e085583dec81c63d19f80a9ba067e38d8043ae"}],"ironic/db/sqlalchemy/api.py":[{"author":{"_account_id":4571,"name":"Steve Baker","email":"sbaker@redhat.com","username":"steve-stevebaker"},"change_message_id":"34f52e42d117660678b696943ccdb0121be6e510","unresolved":true,"context_lines":[{"line_number":85,"context_line":"    return session"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"def _get_node_query_with_all_for_single_node():"},{"line_number":89,"context_line":"    \"\"\"Return a query object for the Node joined with all relevant fields."},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"    This method utilizes a joined load query which creates a result set"}],"source_content_type":"text/x-python","patch_set":4,"id":"11cd9904_4f054571","line":88,"updated":"2022-09-14 23:23:54.000000000","message":"If this is eventually going to be replaced by _get_node_select, maybe some kind of deprecation comment?","commit_id":"da66c3502f996ad15c9ee81201662cc1aa510212"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"84b8d8cb34bf7776ba76878cbd6a928b9f6de313","unresolved":false,"context_lines":[{"line_number":85,"context_line":"    return session"},{"line_number":86,"context_line":""},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"def _get_node_query_with_all_for_single_node():"},{"line_number":89,"context_line":"    \"\"\"Return a query object for the Node joined with all relevant fields."},{"line_number":90,"context_line":""},{"line_number":91,"context_line":"    This method utilizes a joined load query which creates a result set"}],"source_content_type":"text/x-python","patch_set":4,"id":"279bfadb_57435adf","line":88,"in_reply_to":"11cd9904_4f054571","updated":"2022-09-23 19:35:18.000000000","message":"Good point. There is some slightly conflicting language in SQLAlchemy docs. Basically query objects are legacy/deprecated (but no announced removal... I\u0027m going to guess 3.0 or something like that.)\n\nAdded inline note.","commit_id":"da66c3502f996ad15c9ee81201662cc1aa510212"},{"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":"a4f4d75e1e14107b4ecb0f4dfdec2561ec494090","unresolved":true,"context_lines":[{"line_number":140,"context_line":"    # NOTE(TheJulia): This returns a query in the SQLAlchemy 1.4-\u003e2.0"},{"line_number":141,"context_line":"    # migration style as query model loading is deprecated."},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"    # This muist use selectinload to avoid later deduplication."},{"line_number":144,"context_line":"    return (sa.select(models.Node)"},{"line_number":145,"context_line":"            .options(selectinload(models.Node.tags),"},{"line_number":146,"context_line":"                     selectinload(models.Node.traits)))"}],"source_content_type":"text/x-python","patch_set":4,"id":"81e3f299_55874421","line":143,"updated":"2022-09-13 22:45:34.000000000","message":"nit: must","commit_id":"da66c3502f996ad15c9ee81201662cc1aa510212"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"84b8d8cb34bf7776ba76878cbd6a928b9f6de313","unresolved":false,"context_lines":[{"line_number":140,"context_line":"    # NOTE(TheJulia): This returns a query in the SQLAlchemy 1.4-\u003e2.0"},{"line_number":141,"context_line":"    # migration style as query model loading is deprecated."},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"    # This muist use selectinload to avoid later deduplication."},{"line_number":144,"context_line":"    return (sa.select(models.Node)"},{"line_number":145,"context_line":"            .options(selectinload(models.Node.tags),"},{"line_number":146,"context_line":"                     selectinload(models.Node.traits)))"}],"source_content_type":"text/x-python","patch_set":4,"id":"c0f4b006_61ab9ef3","line":143,"in_reply_to":"81e3f299_55874421","updated":"2022-09-23 19:35:18.000000000","message":"Done","commit_id":"da66c3502f996ad15c9ee81201662cc1aa510212"},{"author":{"_account_id":4571,"name":"Steve Baker","email":"sbaker@redhat.com","username":"steve-stevebaker"},"change_message_id":"34f52e42d117660678b696943ccdb0121be6e510","unresolved":true,"context_lines":[{"line_number":146,"context_line":"                     selectinload(models.Node.traits)))"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"def _get_node_query_with_all_for_list():"},{"line_number":150,"context_line":"    \"\"\"Return a query object for the Node with queried extra fields."},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    This method returns a query object joining tags and traits in a pattern"}],"source_content_type":"text/x-python","patch_set":4,"id":"9e8ac6d6_f2e88110","line":149,"updated":"2022-09-14 23:23:54.000000000","message":"It looks like the one actual use of this has been replaced with _get_node_select, and this can be deleted","commit_id":"da66c3502f996ad15c9ee81201662cc1aa510212"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"84b8d8cb34bf7776ba76878cbd6a928b9f6de313","unresolved":false,"context_lines":[{"line_number":146,"context_line":"                     selectinload(models.Node.traits)))"},{"line_number":147,"context_line":""},{"line_number":148,"context_line":""},{"line_number":149,"context_line":"def _get_node_query_with_all_for_list():"},{"line_number":150,"context_line":"    \"\"\"Return a query object for the Node with queried extra fields."},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    This method returns a query object joining tags and traits in a pattern"}],"source_content_type":"text/x-python","patch_set":4,"id":"eefceece_4619df5e","line":149,"in_reply_to":"9e8ac6d6_f2e88110","updated":"2022-09-23 19:35:18.000000000","message":"Done","commit_id":"da66c3502f996ad15c9ee81201662cc1aa510212"},{"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":"a4f4d75e1e14107b4ecb0f4dfdec2561ec494090","unresolved":true,"context_lines":[{"line_number":372,"context_line":"            return query.all()"},{"line_number":373,"context_line":"        else:"},{"line_number":374,"context_line":"            # In this case, we have a sqlalchemy.sql.selectable.Select"},{"line_number":375,"context_line":"            # (most likelly) which utilizes the unified select interface."},{"line_number":376,"context_line":"            res \u003d session.execute(query).fetchall()"},{"line_number":377,"context_line":"            if len(res) \u003d\u003d 0:"},{"line_number":378,"context_line":"                # Return an empty list instead of a class with no objects."}],"source_content_type":"text/x-python","patch_set":4,"id":"2a7efa54_8b4ecc15","line":375,"updated":"2022-09-13 22:45:34.000000000","message":"nit: likely","commit_id":"da66c3502f996ad15c9ee81201662cc1aa510212"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"84b8d8cb34bf7776ba76878cbd6a928b9f6de313","unresolved":false,"context_lines":[{"line_number":372,"context_line":"            return query.all()"},{"line_number":373,"context_line":"        else:"},{"line_number":374,"context_line":"            # In this case, we have a sqlalchemy.sql.selectable.Select"},{"line_number":375,"context_line":"            # (most likelly) which utilizes the unified select interface."},{"line_number":376,"context_line":"            res \u003d session.execute(query).fetchall()"},{"line_number":377,"context_line":"            if len(res) \u003d\u003d 0:"},{"line_number":378,"context_line":"                # Return an empty list instead of a class with no objects."}],"source_content_type":"text/x-python","patch_set":4,"id":"09101af2_60443208","line":375,"in_reply_to":"2a7efa54_8b4ecc15","updated":"2022-09-23 19:35:18.000000000","message":"Done","commit_id":"da66c3502f996ad15c9ee81201662cc1aa510212"},{"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":"a4f4d75e1e14107b4ecb0f4dfdec2561ec494090","unresolved":true,"context_lines":[{"line_number":383,"context_line":"            # Everything is a tuple in a resultset from the unified interface"},{"line_number":384,"context_line":"            # but for objects, our model expects just object access,"},{"line_number":385,"context_line":"            # so we extract and return them."},{"line_number":386,"context_line":"            return [r[0] for r in res]"},{"line_number":387,"context_line":""},{"line_number":388,"context_line":""},{"line_number":389,"context_line":"def _filter_active_conductors(query, interval\u003dNone):"}],"source_content_type":"text/x-python","patch_set":4,"id":"ea9ac839_4abd9faf","line":386,"updated":"2022-09-13 22:45:34.000000000","message":"I don\u0027t know how to make it more efficient, but this seems like it might have a perf impact? But I don\u0027t really know what query.all() was doing under the hood beforehand.","commit_id":"da66c3502f996ad15c9ee81201662cc1aa510212"},{"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":"7d711f96cc7549a5b2a62856b198e34ad3bf2e61","unresolved":false,"context_lines":[{"line_number":383,"context_line":"            # Everything is a tuple in a resultset from the unified interface"},{"line_number":384,"context_line":"            # but for objects, our model expects just object access,"},{"line_number":385,"context_line":"            # so we extract and return them."},{"line_number":386,"context_line":"            return [r[0] for r in res]"},{"line_number":387,"context_line":""},{"line_number":388,"context_line":""},{"line_number":389,"context_line":"def _filter_active_conductors(query, interval\u003dNone):"}],"source_content_type":"text/x-python","patch_set":4,"id":"cf6850f8_4f33d6ba","line":386,"in_reply_to":"93b8e522_9290eba3","updated":"2022-09-14 22:56:57.000000000","message":"Thanks for the information!","commit_id":"da66c3502f996ad15c9ee81201662cc1aa510212"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"0dfadb7d01a26ad10747877af4baed282ff62bd0","unresolved":true,"context_lines":[{"line_number":383,"context_line":"            # Everything is a tuple in a resultset from the unified interface"},{"line_number":384,"context_line":"            # but for objects, our model expects just object access,"},{"line_number":385,"context_line":"            # so we extract and return them."},{"line_number":386,"context_line":"            return [r[0] for r in res]"},{"line_number":387,"context_line":""},{"line_number":388,"context_line":""},{"line_number":389,"context_line":"def _filter_active_conductors(query, interval\u003dNone):"}],"source_content_type":"text/x-python","patch_set":4,"id":"93b8e522_9290eba3","line":386,"in_reply_to":"ea9ac839_4abd9faf","updated":"2022-09-14 13:18:37.000000000","message":"tl;dr query.all basically would do the following:\n1) retreive raw sql response\n2) convert response into a tuple\n3) Map field order into an SQLAlchemy response object\n4) subquery/deduplicate and so on and so forth\n5) convert the response to an modeled object based upon the query.\n\nWe would then convert the object into a dict like object for our ironic objects.\n\nInternally under the hood, deduplicate is gone as the programmer invokes it now if needed, and I believe it goes from 1-\u003e3 which is now a hybrid dict-like object, to 4, and then hands back a tuple on the response type.\n\nOverall, in the grand scheme of the universe, this is likely small potatoes even if it is in net negative territory.","commit_id":"da66c3502f996ad15c9ee81201662cc1aa510212"},{"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":"a4f4d75e1e14107b4ecb0f4dfdec2561ec494090","unresolved":true,"context_lines":[{"line_number":680,"context_line":"            # magically get a changed node as they moved from the"},{"line_number":681,"context_line":"            # many ways to do things to singular ways to do things."},{"line_number":682,"context_line":"            if res.rowcount !\u003d 1:"},{"line_number":683,"context_line":"                # Nothing updated and node exists. Must already be"},{"line_number":684,"context_line":"                # locked."},{"line_number":685,"context_line":"                raise exception.NodeLocked(node\u003dnode.uuid,"},{"line_number":686,"context_line":"                                           host\u003dnode.reservation)"}],"source_content_type":"text/x-python","patch_set":4,"id":"77525f47_4c067ddb","line":683,"updated":"2022-09-13 22:45:34.000000000","message":"I\u0027m confused how, in this case, you can be certain that the node still exists. Isn\u0027t it possible to end up with 0 rows if there is no node with id?","commit_id":"da66c3502f996ad15c9ee81201662cc1aa510212"},{"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":"7d711f96cc7549a5b2a62856b198e34ad3bf2e61","unresolved":false,"context_lines":[{"line_number":680,"context_line":"            # magically get a changed node as they moved from the"},{"line_number":681,"context_line":"            # many ways to do things to singular ways to do things."},{"line_number":682,"context_line":"            if res.rowcount !\u003d 1:"},{"line_number":683,"context_line":"                # Nothing updated and node exists. Must already be"},{"line_number":684,"context_line":"                # locked."},{"line_number":685,"context_line":"                raise exception.NodeLocked(node\u003dnode.uuid,"},{"line_number":686,"context_line":"                                           host\u003dnode.reservation)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7c39c927_53155645","line":683,"in_reply_to":"5c89671c_9359d691","updated":"2022-09-14 22:56:57.000000000","message":"This makes sense, ty.","commit_id":"da66c3502f996ad15c9ee81201662cc1aa510212"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"0dfadb7d01a26ad10747877af4baed282ff62bd0","unresolved":true,"context_lines":[{"line_number":680,"context_line":"            # magically get a changed node as they moved from the"},{"line_number":681,"context_line":"            # many ways to do things to singular ways to do things."},{"line_number":682,"context_line":"            if res.rowcount !\u003d 1:"},{"line_number":683,"context_line":"                # Nothing updated and node exists. Must already be"},{"line_number":684,"context_line":"                # locked."},{"line_number":685,"context_line":"                raise exception.NodeLocked(node\u003dnode.uuid,"},{"line_number":686,"context_line":"                                           host\u003dnode.reservation)"}],"source_content_type":"text/x-python","patch_set":4,"id":"5c89671c_9359d691","line":683,"in_reply_to":"77525f47_4c067ddb","updated":"2022-09-14 13:18:37.000000000","message":"you get 0 rows updated if the node is already locked and the value already matches, in addition to the query just not matching upfront. The prior code did basically the same thing.","commit_id":"da66c3502f996ad15c9ee81201662cc1aa510212"},{"author":{"_account_id":4571,"name":"Steve Baker","email":"sbaker@redhat.com","username":"steve-stevebaker"},"change_message_id":"34f52e42d117660678b696943ccdb0121be6e510","unresolved":true,"context_lines":[{"line_number":740,"context_line":"            except NoResultFound:"},{"line_number":741,"context_line":"                raise exception.NodeNotFound(node\u003dnode_id)"},{"line_number":742,"context_line":"            # Return a node object as that is the contract for this method."},{"line_number":743,"context_line":"            return node"},{"line_number":744,"context_line":""},{"line_number":745,"context_line":"        with _session_for_write() as session, session.begin():"},{"line_number":746,"context_line":"            query \u003d model_query(models.Node)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7256165f_b50d2317","line":743,"updated":"2022-09-14 23:23:54.000000000","message":"I don\u0027t see any return in the old method","commit_id":"da66c3502f996ad15c9ee81201662cc1aa510212"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"84b8d8cb34bf7776ba76878cbd6a928b9f6de313","unresolved":false,"context_lines":[{"line_number":740,"context_line":"            except NoResultFound:"},{"line_number":741,"context_line":"                raise exception.NodeNotFound(node\u003dnode_id)"},{"line_number":742,"context_line":"            # Return a node object as that is the contract for this method."},{"line_number":743,"context_line":"            return node"},{"line_number":744,"context_line":""},{"line_number":745,"context_line":"        with _session_for_write() as session, session.begin():"},{"line_number":746,"context_line":"            query \u003d model_query(models.Node)"}],"source_content_type":"text/x-python","patch_set":4,"id":"ef42d020_0de68d02","line":743,"in_reply_to":"7256165f_b50d2317","updated":"2022-09-23 19:35:18.000000000","message":"no, no there was not. Great catch!","commit_id":"da66c3502f996ad15c9ee81201662cc1aa510212"},{"author":{"_account_id":4571,"name":"Steve Baker","email":"sbaker@redhat.com","username":"steve-stevebaker"},"change_message_id":"34f52e42d117660678b696943ccdb0121be6e510","unresolved":true,"context_lines":[{"line_number":742,"context_line":"            # Return a node object as that is the contract for this method."},{"line_number":743,"context_line":"            return node"},{"line_number":744,"context_line":""},{"line_number":745,"context_line":"        with _session_for_write() as session, session.begin():"},{"line_number":746,"context_line":"            query \u003d model_query(models.Node)"},{"line_number":747,"context_line":"            query \u003d add_identity_filter(query, node_id)"},{"line_number":748,"context_line":""},{"line_number":749,"context_line":"    @oslo_db_api.retry_on_deadlock"},{"line_number":750,"context_line":"    def create_node(self, values):"}],"source_content_type":"text/x-python","patch_set":4,"id":"f056f584_e38ee6cc","line":747,"range":{"start_line":745,"start_character":0,"end_line":747,"end_character":55},"updated":"2022-09-14 23:23:54.000000000","message":"What is the purpose of these 3 lines?","commit_id":"da66c3502f996ad15c9ee81201662cc1aa510212"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"84b8d8cb34bf7776ba76878cbd6a928b9f6de313","unresolved":false,"context_lines":[{"line_number":742,"context_line":"            # Return a node object as that is the contract for this method."},{"line_number":743,"context_line":"            return node"},{"line_number":744,"context_line":""},{"line_number":745,"context_line":"        with _session_for_write() as session, session.begin():"},{"line_number":746,"context_line":"            query \u003d model_query(models.Node)"},{"line_number":747,"context_line":"            query \u003d add_identity_filter(query, node_id)"},{"line_number":748,"context_line":""},{"line_number":749,"context_line":"    @oslo_db_api.retry_on_deadlock"},{"line_number":750,"context_line":"    def create_node(self, values):"}],"source_content_type":"text/x-python","patch_set":4,"id":"e9db6402_6d5acb6a","line":747,"range":{"start_line":745,"start_character":0,"end_line":747,"end_character":55},"in_reply_to":"f056f584_e38ee6cc","updated":"2022-09-23 19:35:18.000000000","message":"likely cruft... I\u0027m going to remove them.","commit_id":"da66c3502f996ad15c9ee81201662cc1aa510212"},{"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":"a4f4d75e1e14107b4ecb0f4dfdec2561ec494090","unresolved":true,"context_lines":[{"line_number":791,"context_line":""},{"line_number":792,"context_line":"    def _get_node_by_id_no_joins(self, node_id):"},{"line_number":793,"context_line":"        # TODO(TheJulia): Maybe replace with this with a minimal"},{"line_number":794,"context_line":"        # \"get these three fields\" thing."},{"line_number":795,"context_line":"        try:"},{"line_number":796,"context_line":"            with _session_for_read() as session:"},{"line_number":797,"context_line":"                return session.execute("}],"source_content_type":"text/x-python","patch_set":4,"id":"0e1a56b9_82fff4cf","line":794,"updated":"2022-09-13 22:45:34.000000000","message":"Hmm. I\u0027m not sure how returning fewer fields here could have a measurable performance impact, unless you\u0027ve got a crazy slow DB or lots of ram pressure.","commit_id":"da66c3502f996ad15c9ee81201662cc1aa510212"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"0dfadb7d01a26ad10747877af4baed282ff62bd0","unresolved":true,"context_lines":[{"line_number":791,"context_line":""},{"line_number":792,"context_line":"    def _get_node_by_id_no_joins(self, node_id):"},{"line_number":793,"context_line":"        # TODO(TheJulia): Maybe replace with this with a minimal"},{"line_number":794,"context_line":"        # \"get these three fields\" thing."},{"line_number":795,"context_line":"        try:"},{"line_number":796,"context_line":"            with _session_for_read() as session:"},{"line_number":797,"context_line":"                return session.execute("}],"source_content_type":"text/x-python","patch_set":4,"id":"67b30a63_0baf6ef3","line":794,"in_reply_to":"0e1a56b9_82fff4cf","updated":"2022-09-14 13:18:37.000000000","message":"For a single node, it is not a huge deal, *but* fewer fields means less looping in sqlalchemy and every object transition/conversion above it.","commit_id":"da66c3502f996ad15c9ee81201662cc1aa510212"},{"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":"7d711f96cc7549a5b2a62856b198e34ad3bf2e61","unresolved":true,"context_lines":[{"line_number":791,"context_line":""},{"line_number":792,"context_line":"    def _get_node_by_id_no_joins(self, node_id):"},{"line_number":793,"context_line":"        # TODO(TheJulia): Maybe replace with this with a minimal"},{"line_number":794,"context_line":"        # \"get these three fields\" thing."},{"line_number":795,"context_line":"        try:"},{"line_number":796,"context_line":"            with _session_for_read() as session:"},{"line_number":797,"context_line":"                return session.execute("}],"source_content_type":"text/x-python","patch_set":4,"id":"d6d139d1_7c3056ae","line":794,"in_reply_to":"67b30a63_0baf6ef3","updated":"2022-09-14 22:56:57.000000000","message":"That makes sense, I figured there was a benefit somewhere I wasn\u0027t thinking of.","commit_id":"da66c3502f996ad15c9ee81201662cc1aa510212"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"84b8d8cb34bf7776ba76878cbd6a928b9f6de313","unresolved":false,"context_lines":[{"line_number":791,"context_line":""},{"line_number":792,"context_line":"    def _get_node_by_id_no_joins(self, node_id):"},{"line_number":793,"context_line":"        # TODO(TheJulia): Maybe replace with this with a minimal"},{"line_number":794,"context_line":"        # \"get these three fields\" thing."},{"line_number":795,"context_line":"        try:"},{"line_number":796,"context_line":"            with _session_for_read() as session:"},{"line_number":797,"context_line":"                return session.execute("}],"source_content_type":"text/x-python","patch_set":4,"id":"461aa69c_821157c3","line":794,"in_reply_to":"d6d139d1_7c3056ae","updated":"2022-09-23 19:35:18.000000000","message":"No worries, unfortunately it is something I\u0027m acutely aware of as I\u0027ve spent time studying the time which piles on after the database.","commit_id":"da66c3502f996ad15c9ee81201662cc1aa510212"}],"ironic/db/sqlalchemy/models.py":[{"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":"610c33f4cad9691cdb53d14acfd3c6917f80e8ee","unresolved":true,"context_lines":[{"line_number":228,"context_line":"    # as it causes a subselect to occur, skipping the need for deduplication"},{"line_number":229,"context_line":"    # in general. This puts a slightly higher query load on the DB server, but"},{"line_number":230,"context_line":"    # means *far* less gets shipped over the wire in the end."},{"line_number":231,"context_line":"    traits: orm.Mapped[\"List[\u0027NodeTrait\u0027]\"] \u003d orm.relationship(  # noqa"},{"line_number":232,"context_line":"        \"NodeTrait\","},{"line_number":233,"context_line":"        back_populates\u003d\"node\","},{"line_number":234,"context_line":"        lazy\u003d\"selectin\")"}],"source_content_type":"text/x-python","patch_set":4,"id":"26548e2d_e54ceec9","line":231,"updated":"2022-09-15 19:16:02.000000000","message":"sqlalchemy 2.0 beta is choking on this relationship def, see https://gist.github.com/jayofdoom/cb132e3d39213eb79b195e849bfbcebb","commit_id":"da66c3502f996ad15c9ee81201662cc1aa510212"},{"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":"629142768070794433ef5d070bd4a14573bb9889","unresolved":true,"context_lines":[{"line_number":228,"context_line":"    # as it causes a subselect to occur, skipping the need for deduplication"},{"line_number":229,"context_line":"    # in general. This puts a slightly higher query load on the DB server, but"},{"line_number":230,"context_line":"    # means *far* less gets shipped over the wire in the end."},{"line_number":231,"context_line":"    traits: orm.Mapped[\"List[\u0027NodeTrait\u0027]\"] \u003d orm.relationship(  # noqa"},{"line_number":232,"context_line":"        \"NodeTrait\","},{"line_number":233,"context_line":"        back_populates\u003d\"node\","},{"line_number":234,"context_line":"        lazy\u003d\"selectin\")"}],"source_content_type":"text/x-python","patch_set":4,"id":"66845b0e_dee45432","line":231,"in_reply_to":"26548e2d_e54ceec9","updated":"2022-09-15 19:54:30.000000000","message":"This is fixed. Testing with sqlalchemy 2.0 and the first 2 phases of patches showed a 28 nodes/second improvement (we fetched 100k nodes from the DB ~4 seconds faster).","commit_id":"da66c3502f996ad15c9ee81201662cc1aa510212"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"84b8d8cb34bf7776ba76878cbd6a928b9f6de313","unresolved":false,"context_lines":[{"line_number":228,"context_line":"    # as it causes a subselect to occur, skipping the need for deduplication"},{"line_number":229,"context_line":"    # in general. This puts a slightly higher query load on the DB server, but"},{"line_number":230,"context_line":"    # means *far* less gets shipped over the wire in the end."},{"line_number":231,"context_line":"    traits: orm.Mapped[\"List[\u0027NodeTrait\u0027]\"] \u003d orm.relationship(  # noqa"},{"line_number":232,"context_line":"        \"NodeTrait\","},{"line_number":233,"context_line":"        back_populates\u003d\"node\","},{"line_number":234,"context_line":"        lazy\u003d\"selectin\")"}],"source_content_type":"text/x-python","patch_set":4,"id":"6de26847_d5e17fd9","line":231,"in_reply_to":"66845b0e_dee45432","updated":"2022-09-23 19:35:18.000000000","message":"Ack","commit_id":"da66c3502f996ad15c9ee81201662cc1aa510212"}]}
