)]}'
{"ovsdbapp/backend/ovs_idl/idlutils.py":[{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"607f5d88502002733379d36a6ef65f7dbfefe36d","unresolved":true,"context_lines":[{"line_number":469,"context_line":"                # Empty, return default."},{"line_number":470,"context_line":"                pass"},{"line_number":471,"context_line":"        elif (isinstance(value, abc.Iterable) and not isinstance(value, str)):"},{"line_number":472,"context_line":"            try:"},{"line_number":473,"context_line":"                # Pass through early without iterating if not Rows."},{"line_number":474,"context_line":"                if value and isinstance(value[0], idl.Row):"},{"line_number":475,"context_line":"                    return type(value)(str(idl._row_to_uuid(x)) for x in value)"}],"source_content_type":"text/x-python","patch_set":1,"id":"b839b1a1_aac3dc0c","line":472,"updated":"2025-06-18 22:15:49.000000000","message":"Should this wrap only L475?","commit_id":"d38ed3431edf1de5d6e6530ea9aa09cacb1725a3"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"0b8d2a798d402f8448a4c1c37783607ae77026ad","unresolved":false,"context_lines":[{"line_number":469,"context_line":"                # Empty, return default."},{"line_number":470,"context_line":"                pass"},{"line_number":471,"context_line":"        elif (isinstance(value, abc.Iterable) and not isinstance(value, str)):"},{"line_number":472,"context_line":"            try:"},{"line_number":473,"context_line":"                # Pass through early without iterating if not Rows."},{"line_number":474,"context_line":"                if value and isinstance(value[0], idl.Row):"},{"line_number":475,"context_line":"                    return type(value)(str(idl._row_to_uuid(x)) for x in value)"}],"source_content_type":"text/x-python","patch_set":1,"id":"6fdbf6b8_b796f525","line":472,"in_reply_to":"3dcb099d_1a85e0aa","updated":"2025-06-21 15:07:57.000000000","message":"\u003e I normally don\u0027t like __str__() tests because they\u0027re informational and them changing formats isn\u0027t usually \"wrong\" and testing the output matching directly means changing the tests every time you make a change (which as you know, I will rant for hours about). But, you\u0027re right, I absolutely should break out the nested function into something testable.\n\nWell the function has some inputs and produces specific output format of str type. imho it is fine to update the unittests if the output format changes because it\u0027s a change of a function output and doesn\u0027t rely on the implementation.","commit_id":"d38ed3431edf1de5d6e6530ea9aa09cacb1725a3"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"b996869f6ed0d5c9d45ba21919d3732bef74bea8","unresolved":false,"context_lines":[{"line_number":469,"context_line":"                # Empty, return default."},{"line_number":470,"context_line":"                pass"},{"line_number":471,"context_line":"        elif (isinstance(value, abc.Iterable) and not isinstance(value, str)):"},{"line_number":472,"context_line":"            try:"},{"line_number":473,"context_line":"                # Pass through early without iterating if not Rows."},{"line_number":474,"context_line":"                if value and isinstance(value[0], idl.Row):"},{"line_number":475,"context_line":"                    return type(value)(str(idl._row_to_uuid(x)) for x in value)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3dcb099d_1a85e0aa","line":472,"in_reply_to":"7bc7f4d0_18540824","updated":"2025-06-20 13:42:09.000000000","message":"I guess I could change the `isinstance(x, Row)` calls to `hasattr(x, \u0027uuid\u0027)` calls and lean into the abomination of duck typing.","commit_id":"d38ed3431edf1de5d6e6530ea9aa09cacb1725a3"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"6f9ea5409a71073e83eab459a646425bed25a2a0","unresolved":false,"context_lines":[{"line_number":469,"context_line":"                # Empty, return default."},{"line_number":470,"context_line":"                pass"},{"line_number":471,"context_line":"        elif (isinstance(value, abc.Iterable) and not isinstance(value, str)):"},{"line_number":472,"context_line":"            try:"},{"line_number":473,"context_line":"                # Pass through early without iterating if not Rows."},{"line_number":474,"context_line":"                if value and isinstance(value[0], idl.Row):"},{"line_number":475,"context_line":"                    return type(value)(str(idl._row_to_uuid(x)) for x in value)"}],"source_content_type":"text/x-python","patch_set":1,"id":"da840188_1933f6dc","line":472,"in_reply_to":"b839b1a1_aac3dc0c","updated":"2025-06-18 22:18:40.000000000","message":"Good eye! You started reviewing before I could upload my change. :)  (the extra parens were from the original patch where collections.abc made the line longer)","commit_id":"d38ed3431edf1de5d6e6530ea9aa09cacb1725a3"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"116f5b54a8e34f66febc5f5200657e61194f2616","unresolved":false,"context_lines":[{"line_number":469,"context_line":"                # Empty, return default."},{"line_number":470,"context_line":"                pass"},{"line_number":471,"context_line":"        elif (isinstance(value, abc.Iterable) and not isinstance(value, str)):"},{"line_number":472,"context_line":"            try:"},{"line_number":473,"context_line":"                # Pass through early without iterating if not Rows."},{"line_number":474,"context_line":"                if value and isinstance(value[0], idl.Row):"},{"line_number":475,"context_line":"                    return type(value)(str(idl._row_to_uuid(x)) for x in value)"}],"source_content_type":"text/x-python","patch_set":1,"id":"f6d8ae10_8a408575","line":472,"in_reply_to":"d194e0b5_7a7b3866","updated":"2025-06-19 22:37:19.000000000","message":"Sounds like something unittests would find :)","commit_id":"d38ed3431edf1de5d6e6530ea9aa09cacb1725a3"},{"author":{"_account_id":8655,"name":"Jakub Libosvar","email":"libosvar@redhat.com","username":"jlibosva"},"change_message_id":"e0e5e6ba78cd5169ca91b995f65c04ce027b2002","unresolved":false,"context_lines":[{"line_number":469,"context_line":"                # Empty, return default."},{"line_number":470,"context_line":"                pass"},{"line_number":471,"context_line":"        elif (isinstance(value, abc.Iterable) and not isinstance(value, str)):"},{"line_number":472,"context_line":"            try:"},{"line_number":473,"context_line":"                # Pass through early without iterating if not Rows."},{"line_number":474,"context_line":"                if value and isinstance(value[0], idl.Row):"},{"line_number":475,"context_line":"                    return type(value)(str(idl._row_to_uuid(x)) for x in value)"}],"source_content_type":"text/x-python","patch_set":1,"id":"f08bd38e_f0d9a547","line":472,"in_reply_to":"da840188_1933f6dc","updated":"2025-06-19 19:18:44.000000000","message":"I didn\u0027t notice the parens :) I meant if the try block should only be around that line, inside of the if-clause as it seems it can\u0027t be raised from the condition","commit_id":"d38ed3431edf1de5d6e6530ea9aa09cacb1725a3"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"aea457fcd0318f563508222dc65a01ad3a03cabc","unresolved":false,"context_lines":[{"line_number":469,"context_line":"                # Empty, return default."},{"line_number":470,"context_line":"                pass"},{"line_number":471,"context_line":"        elif (isinstance(value, abc.Iterable) and not isinstance(value, str)):"},{"line_number":472,"context_line":"            try:"},{"line_number":473,"context_line":"                # Pass through early without iterating if not Rows."},{"line_number":474,"context_line":"                if value and isinstance(value[0], idl.Row):"},{"line_number":475,"context_line":"                    return type(value)(str(idl._row_to_uuid(x)) for x in value)"}],"source_content_type":"text/x-python","patch_set":1,"id":"d194e0b5_7a7b3866","line":472,"in_reply_to":"f08bd38e_f0d9a547","updated":"2025-06-19 22:24:36.000000000","message":"It\u0027s actually a real bug, but *because* the condition can raise a `TypeError`. I use `Iterable` even though current code would only ever be a `list` because in the future I would really prefer that set-type columns actually return a `set`. So if `value` is a `set`, `value[0]` _would_ raise a `TypeError` and we\u0027d not convert a `set` of `Row` objects to a `set` of UUID strings.","commit_id":"d38ed3431edf1de5d6e6530ea9aa09cacb1725a3"},{"author":{"_account_id":5756,"name":"Terry Wilson","email":"twilson@redhat.com","username":"otherwiseguy"},"change_message_id":"8271b5736ea90b363926e8f1b38a1959de22c0bc","unresolved":false,"context_lines":[{"line_number":469,"context_line":"                # Empty, return default."},{"line_number":470,"context_line":"                pass"},{"line_number":471,"context_line":"        elif (isinstance(value, abc.Iterable) and not isinstance(value, str)):"},{"line_number":472,"context_line":"            try:"},{"line_number":473,"context_line":"                # Pass through early without iterating if not Rows."},{"line_number":474,"context_line":"                if value and isinstance(value[0], idl.Row):"},{"line_number":475,"context_line":"                    return type(value)(str(idl._row_to_uuid(x)) for x in value)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7bc7f4d0_18540824","line":472,"in_reply_to":"f6d8ae10_8a408575","updated":"2025-06-20 13:27:13.000000000","message":"I normally don\u0027t like `__str__()` tests because they\u0027re informational and them changing formats isn\u0027t usually \"wrong\" and testing the output matching directly means changing the tests every time you make a change (which as you know, I will rant for hours about). But, you\u0027re right, I absolutely should break out the nested function into something testable.\n\nBut also, dealing with `Row` objects in unit tests is tricky given how tied they are to the Idl, schemas, tables, and all of the `__getattr__()` stuff, especially with `isinstance` checks in the code being tested. 😭 So I\u0027m not sure if it\u0027ll be worth it.","commit_id":"d38ed3431edf1de5d6e6530ea9aa09cacb1725a3"}]}