)]}'
{"osc_lib/utils/__init__.py":[{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"baae959c15279a20a06dbcd73173f55072d2289b","unresolved":true,"context_lines":[{"line_number":723,"context_line":"              columns"},{"line_number":724,"context_line":"    \"\"\""},{"line_number":725,"context_line":""},{"line_number":726,"context_line":"    if getattr(sdk_resource, \u0027allow_fetch\u0027, None) is not None:"},{"line_number":727,"context_line":"        resource_dict \u003d sdk_resource.to_dict("},{"line_number":728,"context_line":"            body\u003dTrue, headers\u003dFalse, ignore_none\u003dFalse)"},{"line_number":729,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"2a89825a_b3aa9d62","line":726,"updated":"2022-01-17 10:27:52.000000000","message":"So IIUC you are using these attributes just as a proxy in order to detect whether we have a resource.Resource object or a simple dict? \n\nWhy not be explicit about it and use isinstance()? Or check whether the to_dict function that we want to call is present? Or even call to_dict unconditionally, but within a try/except clause?\n\nAll of this would seem cleaner to me than the above. If I miss something and there is in fact a good reason to do this, some explaning comment might be useful.","commit_id":"a4b08427f551addeae05fc7b75cc6e5a0de413a0"},{"author":{"_account_id":27900,"name":"Artem Goncharov","email":"artem.goncharov@gmail.com","username":"gtema"},"change_message_id":"1e3ab7186d06abdd6497d3cfacf3556e38ae9636","unresolved":true,"context_lines":[{"line_number":723,"context_line":"              columns"},{"line_number":724,"context_line":"    \"\"\""},{"line_number":725,"context_line":""},{"line_number":726,"context_line":"    if getattr(sdk_resource, \u0027allow_fetch\u0027, None) is not None:"},{"line_number":727,"context_line":"        resource_dict \u003d sdk_resource.to_dict("},{"line_number":728,"context_line":"            body\u003dTrue, headers\u003dFalse, ignore_none\u003dFalse)"},{"line_number":729,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"4c920a8a_9cd4611f","line":726,"in_reply_to":"2a89825a_b3aa9d62","updated":"2022-01-17 10:34:12.000000000","message":"just updated it without thinking too much. Idea with try/except seems reasonable","commit_id":"a4b08427f551addeae05fc7b75cc6e5a0de413a0"},{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"960596cb17491f4c72371b1310f4b965cfd52e42","unresolved":true,"context_lines":[{"line_number":726,"context_line":"    if getattr(sdk_resource, \u0027allow_fetch\u0027, None) is not None:"},{"line_number":727,"context_line":"        # OSC at the moment lands here with FakeResource objects which are not"},{"line_number":728,"context_line":"        # 100% sdk compatible. Unless we introduce SDK test/fake resources we"},{"line_number":729,"context_line":"        # should check presence of the specific method"},{"line_number":730,"context_line":"        resource_dict \u003d sdk_resource.to_dict("},{"line_number":731,"context_line":"            body\u003dTrue, headers\u003dFalse, ignore_none\u003dFalse)"},{"line_number":732,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":5,"id":"595e5eb2_5738a69c","line":729,"updated":"2022-01-18 15:24:26.000000000","message":"Not sure why you reverted the try/except approach, with the correct exception to catch it did seem to work for me.\n\nAnyway, this comment now would seem to indicate that you want to check for the presence of to_dict instead of allow_fetch.","commit_id":"650795af70f8709292200f677f6ee29880aab54e"},{"author":{"_account_id":13252,"name":"Dr. Jens Harbott","display_name":"Jens Harbott (frickler)","email":"frickler@offenerstapel.de","username":"jrosenboom"},"change_message_id":"6ecab758c2ab13aff12b9023d1c488170efc0cbd","unresolved":false,"context_lines":[{"line_number":726,"context_line":"    if getattr(sdk_resource, \u0027allow_fetch\u0027, None) is not None:"},{"line_number":727,"context_line":"        # OSC at the moment lands here with FakeResource objects which are not"},{"line_number":728,"context_line":"        # 100% sdk compatible. Unless we introduce SDK test/fake resources we"},{"line_number":729,"context_line":"        # should check presence of the specific method"},{"line_number":730,"context_line":"        resource_dict \u003d sdk_resource.to_dict("},{"line_number":731,"context_line":"            body\u003dTrue, headers\u003dFalse, ignore_none\u003dFalse)"},{"line_number":732,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":5,"id":"444c825c_ca8f3fbd","line":729,"in_reply_to":"13675f8c_97436990","updated":"2022-01-19 08:46:03.000000000","message":"My solution would have been to simply also catch the TypeError in the except, but I guess it\u0027s fine this way for now.","commit_id":"650795af70f8709292200f677f6ee29880aab54e"},{"author":{"_account_id":27900,"name":"Artem Goncharov","email":"artem.goncharov@gmail.com","username":"gtema"},"change_message_id":"e9c086c2dddb426ad9c3d86cad918ab8296be856","unresolved":true,"context_lines":[{"line_number":726,"context_line":"    if getattr(sdk_resource, \u0027allow_fetch\u0027, None) is not None:"},{"line_number":727,"context_line":"        # OSC at the moment lands here with FakeResource objects which are not"},{"line_number":728,"context_line":"        # 100% sdk compatible. Unless we introduce SDK test/fake resources we"},{"line_number":729,"context_line":"        # should check presence of the specific method"},{"line_number":730,"context_line":"        resource_dict \u003d sdk_resource.to_dict("},{"line_number":731,"context_line":"            body\u003dTrue, headers\u003dFalse, ignore_none\u003dFalse)"},{"line_number":732,"context_line":"    else:"}],"source_content_type":"text/x-python","patch_set":5,"id":"13675f8c_97436990","line":729,"in_reply_to":"595e5eb2_5738a69c","updated":"2022-01-18 17:46:35.000000000","message":"well, it\u0027s pretty complex now. Biggest issue is that OSC has tons of fake resource objects, that do have to_dict method, but which is not compatible. And this also failed in the try/catch attempt. You can see in https://storage.gra.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_a7d/824909/3/check/osc-tox-py36-tips/a7d2202/testr_results.html (176 failed tests).\n\nIdeally I would like in future to start using real sdk resources in the tests of osc. Then this can be changed to try/except case (well, once this is done there would be no need for try/except at all and we can directly call to_dict)","commit_id":"650795af70f8709292200f677f6ee29880aab54e"}]}