)]}'
{"ironic/tests/unit/objects/utils.py":[{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"338081227b7c2770f9534ffb5f0c74646b507ba9","unresolved":false,"context_lines":[{"line_number":230,"context_line":"    payloads \u003d []"},{"line_number":231,"context_line":"    for (name, payload) in inspect.getmembers(from_module, inspect.isclass):"},{"line_number":232,"context_line":"        if name.endswith(\"Payload\"):"},{"line_number":233,"context_line":"            parent \u003d payload.__bases__[0]"},{"line_number":234,"context_line":"            if (not hasattr(parent, \u0027SCHEMA\u0027) or"},{"line_number":235,"context_line":"                parent.SCHEMA !\u003d payload.SCHEMA):"},{"line_number":236,"context_line":"                payloads.append(payload)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_207eb6ae","line":233,"updated":"2017-01-09 23:22:53.000000000","message":"A comment here might be nice. What does this do?\n\nAfter about 3 minutes of Googling I\u0027m not sure.","commit_id":"431762b2d6a35aa335a3da20bf747bdacbbf6a89"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"034c66ceaa0cd76f45166f4b8ad9a552eb33e143","unresolved":false,"context_lines":[{"line_number":230,"context_line":"    payloads \u003d []"},{"line_number":231,"context_line":"    for (name, payload) in inspect.getmembers(from_module, inspect.isclass):"},{"line_number":232,"context_line":"        if name.endswith(\"Payload\"):"},{"line_number":233,"context_line":"            parent \u003d payload.__bases__[0]"},{"line_number":234,"context_line":"            if (not hasattr(parent, \u0027SCHEMA\u0027) or"},{"line_number":235,"context_line":"                parent.SCHEMA !\u003d payload.SCHEMA):"},{"line_number":236,"context_line":"                payloads.append(payload)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_62afc2e3","line":233,"in_reply_to":"ba5201f7_202cf6aa","updated":"2017-01-10 14:36:55.000000000","message":"Just goes to show, Googling isn\u0027t always efficient if you don\u0027t know what search terms to use :)\n\nIt is unclear where to draw the line between what Python we expect our developers to know, versus what should be documented, but since you asked...","commit_id":"431762b2d6a35aa335a3da20bf747bdacbbf6a89"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"60b65738ef2921fd9bfc60a292df1b974c0c4800","unresolved":false,"context_lines":[{"line_number":230,"context_line":"    payloads \u003d []"},{"line_number":231,"context_line":"    for (name, payload) in inspect.getmembers(from_module, inspect.isclass):"},{"line_number":232,"context_line":"        if name.endswith(\"Payload\"):"},{"line_number":233,"context_line":"            parent \u003d payload.__bases__[0]"},{"line_number":234,"context_line":"            if (not hasattr(parent, \u0027SCHEMA\u0027) or"},{"line_number":235,"context_line":"                parent.SCHEMA !\u003d payload.SCHEMA):"},{"line_number":236,"context_line":"                payloads.append(payload)"}],"source_content_type":"text/x-python","patch_set":1,"id":"ba5201f7_202cf6aa","line":233,"in_reply_to":"ba5201f7_207eb6ae","updated":"2017-01-09 23:25:30.000000000","message":"Oh, I found:\n\nhttps://docs.python.org/2/reference/datamodel.html\n\n__bases__ is a tuple (possibly empty or a singleton) containing the base classes, in the order of their occurrence in the base class list;\n\nAnd: https://docs.python.org/2/library/stdtypes.html#class.__bases__","commit_id":"431762b2d6a35aa335a3da20bf747bdacbbf6a89"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"d2295f2f8b45dc3fd96c16266bc20c068de06cc4","unresolved":false,"context_lines":[{"line_number":230,"context_line":"    payloads \u003d []"},{"line_number":231,"context_line":"    for (name, payload) in inspect.getmembers(from_module, inspect.isclass):"},{"line_number":232,"context_line":"        if name.endswith(\"Payload\"):"},{"line_number":233,"context_line":"            parent \u003d payload.__bases__[0]"},{"line_number":234,"context_line":"            if (not hasattr(parent, \u0027SCHEMA\u0027) or"},{"line_number":235,"context_line":"                parent.SCHEMA !\u003d payload.SCHEMA):"},{"line_number":236,"context_line":"                payloads.append(payload)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a3c09a3_10f5ef55","line":233,"in_reply_to":"ba5201f7_62afc2e3","updated":"2017-01-17 18:32:15.000000000","message":"What if parent with schema is second or third class in this tuple? Might be better to use inspect.getmro, go through this list, get the first class containing the schema, and use it as parent.","commit_id":"431762b2d6a35aa335a3da20bf747bdacbbf6a89"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"d2295f2f8b45dc3fd96c16266bc20c068de06cc4","unresolved":false,"context_lines":[{"line_number":232,"context_line":"        if name.endswith(\"Payload\"):"},{"line_number":233,"context_line":"            parent \u003d payload.__bases__[0]"},{"line_number":234,"context_line":"            if (not hasattr(parent, \u0027SCHEMA\u0027) or"},{"line_number":235,"context_line":"                parent.SCHEMA !\u003d payload.SCHEMA):"},{"line_number":236,"context_line":"                payloads.append(payload)"},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"    return payloads"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a3c09a3_8bf8d0c4","line":235,"range":{"start_line":235,"start_character":33,"end_line":235,"end_character":47},"updated":"2017-01-17 18:32:15.000000000","message":"You ensure that parent has schema, but don\u0027t do this for payload, why? As this test might start failing if one adds some AdditionalPayload that is not actually a payload class but some arbitrary thing.","commit_id":"431762b2d6a35aa335a3da20bf747bdacbbf6a89"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"c913a08d97b5c992341a0b8334c0ba89e33eb0f2","unresolved":false,"context_lines":[{"line_number":227,"context_line":""},{"line_number":228,"context_line":"    \"\"\""},{"line_number":229,"context_line":"    payloads \u003d []"},{"line_number":230,"context_line":"    for (name, payload) in inspect.getmembers(from_module, inspect.isclass):"},{"line_number":231,"context_line":"        # Assume that Payload class names end in \u0027Payload\u0027."},{"line_number":232,"context_line":"        if name.endswith(\"Payload\"):"},{"line_number":233,"context_line":"            # payload.__bases__ is a tuple containing the base classes of"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a57fde8_a411f8b1","line":230,"range":{"start_line":230,"start_character":7,"end_line":230,"end_character":24},"updated":"2017-01-12 10:43:00.000000000","message":"nit: no parentheses needed","commit_id":"3bd1e2ad911f63aabc02dce6c83467c385eda68a"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"fdf2d82b3876e516d2ca89d62b01dcd55f0ac1e0","unresolved":false,"context_lines":[{"line_number":227,"context_line":""},{"line_number":228,"context_line":"    \"\"\""},{"line_number":229,"context_line":"    payloads \u003d []"},{"line_number":230,"context_line":"    for (name, payload) in inspect.getmembers(from_module, inspect.isclass):"},{"line_number":231,"context_line":"        # Assume that Payload class names end in \u0027Payload\u0027."},{"line_number":232,"context_line":"        if name.endswith(\"Payload\"):"},{"line_number":233,"context_line":"            # payload.__bases__ is a tuple containing the base classes of"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a3c09a3_cb9bad64","line":230,"range":{"start_line":230,"start_character":7,"end_line":230,"end_character":24},"in_reply_to":"9a57fde8_a411f8b1","updated":"2017-01-17 23:07:09.000000000","message":"i actually like the paren but I know, no one else does :-(","commit_id":"3bd1e2ad911f63aabc02dce6c83467c385eda68a"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"e6806bcf8cb4f41f2a6c354f51fe8e558e8bc847","unresolved":false,"context_lines":[{"line_number":234,"context_line":"            # payload. We\u0027re interested in payloads that have their own"},{"line_number":235,"context_line":"            # SCHEMA; these SCHEMAs would be different (not inherited) from"},{"line_number":236,"context_line":"            # their parent class."},{"line_number":237,"context_line":"            parent \u003d payload.__bases__[0]"},{"line_number":238,"context_line":"            if (not hasattr(parent, \u0027SCHEMA\u0027) or"},{"line_number":239,"context_line":"                parent.SCHEMA !\u003d payload.SCHEMA):"},{"line_number":240,"context_line":"                payloads.append(payload)"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"    return payloads"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a3c09a3_61b2e395","line":240,"range":{"start_line":237,"start_character":12,"end_line":240,"end_character":40},"updated":"2017-01-17 19:11:37.000000000","message":"something like this:\n\n if name.endswith(\"Payload\") and hasattr(payload, \u0027SCHEMA\u0027):\n   for parent in inspect.getmro(payload):\n     if (hasattr(parent, \u0027SCHEMA\u0027) and\n             parent.SCHEMA !\u003d payload.SCHEMA):\n         payloads.append(payload)\n         break\n   else:\n     payloads.append(payload)\n\nThis will let to avoid situation when there is the following payload and it will be checked again:\n\n class CustomPayload(dict, NodePayload):\n   # SCHEMA the same as in NodePayload\n   pass\n\nOr when there is some random class with name ending in \"Payload\" not containing SCHEMA at all.","commit_id":"3bd1e2ad911f63aabc02dce6c83467c385eda68a"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"6361d8fb6b1d5f8c6d4ca65c654d6aa0d253e8d6","unresolved":false,"context_lines":[{"line_number":234,"context_line":"            # payload. We\u0027re interested in payloads that have their own"},{"line_number":235,"context_line":"            # SCHEMA; these SCHEMAs would be different (not inherited) from"},{"line_number":236,"context_line":"            # their parent class."},{"line_number":237,"context_line":"            parent \u003d payload.__bases__[0]"},{"line_number":238,"context_line":"            if (not hasattr(parent, \u0027SCHEMA\u0027) or"},{"line_number":239,"context_line":"                parent.SCHEMA !\u003d payload.SCHEMA):"},{"line_number":240,"context_line":"                payloads.append(payload)"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"    return payloads"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a3c09a3_5d46a44f","line":240,"range":{"start_line":237,"start_character":12,"end_line":240,"end_character":40},"in_reply_to":"7a3c09a3_5cdf9068","updated":"2017-01-17 22:43:44.000000000","message":"Yeah, that\u0027d be covered, because hasattr(parent, \u0027SCHEMA\u0027) would be False\n\nThe only scenario I could think of where this type of check might be problematic is when we have a non-notification-related class ending in Payload.\n\nSince that\u0027s not the case today, I think it\u0027s fine if the logic here gets updated if that ever happens.\n\nA way around that could be to use inspect.getmro like this on L232:\n\nif notification_base.NotificationPayloadBase in inspect.getmro(payload):\n...\n\nwhere notification_base.NotificationPayloadBase is this:\n\nhttps://github.com/openstack/ironic/blob/18a1f97d2445f69ce9a689306e64a5213fa27b8f/ironic/objects/notification.py#L129\n\ni think that\u0027d work: http://paste.openstack.org/show/595289/","commit_id":"3bd1e2ad911f63aabc02dce6c83467c385eda68a"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"fdf2d82b3876e516d2ca89d62b01dcd55f0ac1e0","unresolved":false,"context_lines":[{"line_number":234,"context_line":"            # payload. We\u0027re interested in payloads that have their own"},{"line_number":235,"context_line":"            # SCHEMA; these SCHEMAs would be different (not inherited) from"},{"line_number":236,"context_line":"            # their parent class."},{"line_number":237,"context_line":"            parent \u003d payload.__bases__[0]"},{"line_number":238,"context_line":"            if (not hasattr(parent, \u0027SCHEMA\u0027) or"},{"line_number":239,"context_line":"                parent.SCHEMA !\u003d payload.SCHEMA):"},{"line_number":240,"context_line":"                payloads.append(payload)"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"    return payloads"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a3c09a3_dd14544e","line":240,"range":{"start_line":237,"start_character":12,"end_line":240,"end_character":40},"in_reply_to":"7a3c09a3_5d46a44f","updated":"2017-01-17 23:07:09.000000000","message":"I modified it; let me know what you think :)","commit_id":"3bd1e2ad911f63aabc02dce6c83467c385eda68a"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"fd6e39d12785f011e3f5c8a892c6d235ea00f178","unresolved":false,"context_lines":[{"line_number":234,"context_line":"            # payload. We\u0027re interested in payloads that have their own"},{"line_number":235,"context_line":"            # SCHEMA; these SCHEMAs would be different (not inherited) from"},{"line_number":236,"context_line":"            # their parent class."},{"line_number":237,"context_line":"            parent \u003d payload.__bases__[0]"},{"line_number":238,"context_line":"            if (not hasattr(parent, \u0027SCHEMA\u0027) or"},{"line_number":239,"context_line":"                parent.SCHEMA !\u003d payload.SCHEMA):"},{"line_number":240,"context_line":"                payloads.append(payload)"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"    return payloads"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a3c09a3_5cdf9068","line":240,"range":{"start_line":237,"start_character":12,"end_line":240,"end_character":40},"in_reply_to":"7a3c09a3_61b2e395","updated":"2017-01-17 19:16:04.000000000","message":"Oh, so the second case is already covered it seems, sorry :)","commit_id":"3bd1e2ad911f63aabc02dce6c83467c385eda68a"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"4613ec3d34b681e4bdfdf2becf4a782144ae6830","unresolved":false,"context_lines":[{"line_number":239,"context_line":""},{"line_number":240,"context_line":"            # First class is this payload class, parent class is the 2nd"},{"line_number":241,"context_line":"            # one in the tuple"},{"line_number":242,"context_line":"            parent \u003d base_classes[1]"},{"line_number":243,"context_line":"            if (not hasattr(parent, \u0027SCHEMA\u0027) or"},{"line_number":244,"context_line":"                parent.SCHEMA !\u003d payload.SCHEMA):"},{"line_number":245,"context_line":"                payloads.append(payload)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a3c09a3_a4703da5","line":242,"updated":"2017-01-18 08:56:46.000000000","message":"This still will be testing the following class, while there is no schema change:\n\n class CustomPayload(\u003csome mixin class\u003e, NotificationPayloadBase):\n   SCHEMA \u003d NotificationPayloadBase.SCHEMA\n\nMaybe that\u0027s fine tho.","commit_id":"1e162bf622dd943b1c9a9ab2b94d3fb1498911d1"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"e598f9eedd4cfa646ec05b213d53fb1a8599537e","unresolved":false,"context_lines":[{"line_number":239,"context_line":""},{"line_number":240,"context_line":"            # First class is this payload class, parent class is the 2nd"},{"line_number":241,"context_line":"            # one in the tuple"},{"line_number":242,"context_line":"            parent \u003d base_classes[1]"},{"line_number":243,"context_line":"            if (not hasattr(parent, \u0027SCHEMA\u0027) or"},{"line_number":244,"context_line":"                parent.SCHEMA !\u003d payload.SCHEMA):"},{"line_number":245,"context_line":"                payloads.append(payload)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a3c09a3_37a628ae","line":242,"in_reply_to":"7a3c09a3_177e8c69","updated":"2017-01-18 15:52:12.000000000","message":"Even a bit better:\n\n base_classes \u003d inspect.getmro(payload)\n if notification.NotificationPayloadBase not in base_classes:\n   # The class may have the desired name but it isn\u0027t a REAL\n   # Payload class; skip it.\n   continue\n for c in base_classes[1:]:\n   if hasattr(parent, \u0027SCHEMA\u0027):\n     if parent.SCHEMA !\u003d payload.SCHEMA:\n       payloads.append(payload)\n     break","commit_id":"1e162bf622dd943b1c9a9ab2b94d3fb1498911d1"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"a6eddbb461fd068b66f52965378d2af39e04e8fa","unresolved":false,"context_lines":[{"line_number":239,"context_line":""},{"line_number":240,"context_line":"            # First class is this payload class, parent class is the 2nd"},{"line_number":241,"context_line":"            # one in the tuple"},{"line_number":242,"context_line":"            parent \u003d base_classes[1]"},{"line_number":243,"context_line":"            if (not hasattr(parent, \u0027SCHEMA\u0027) or"},{"line_number":244,"context_line":"                parent.SCHEMA !\u003d payload.SCHEMA):"},{"line_number":245,"context_line":"                payloads.append(payload)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a3c09a3_1d53ff6d","line":242,"in_reply_to":"7a3c09a3_37a628ae","updated":"2017-01-18 19:18:12.000000000","message":"I\u0027m not sure why, but I am having a hard time following this; I feel like it is getting more complex for not much gain.\n\nI have to admit, I didn\u0027t realize that the base Payload class NotificationPayloadBase has a .SCHEMA. Given that its value is empty, the code will work, but if it wasn\u0027t empty, I am not sure it would work.\n\nwrt the example:\n  class CustomPayload(\u003csome mixin class\u003e, NotificationPayloadBase):\n   SCHEMA \u003d NotificationPayloadBase.SCHEMA\n\nI think you mean that there could be some ParentOfCustomPayload with SCHEMA different from NotificationPayloadBase.SCHEMA. In which case, this code would pick the CustomPayload. Whereas you don\u0027t think it should. This CustomePayload DOES satisfy the criterion I was looking for -- that it has a .SCHEMA defined. (even though it is defined to be the same as another class\u0027 .SCHEMA.)\n\nI\u0027d rather leave the code as is because I don\u0027t see how the latest proposal above addresses the example, it looks to me like CustomPayload would still get picked, becuase ParentOfCustomPayload.SCHEMA is diff from CustomPayload.SCHEMA. I might be wrong about the example but I question whether it is worth me spending more time on this than trying to land some higher priority patches into Ocata, and Vladyslav is ok with the existing code :)","commit_id":"1e162bf622dd943b1c9a9ab2b94d3fb1498911d1"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"97fb2e2ff7b0109118315d6f8e9c834e6d49ab79","unresolved":false,"context_lines":[{"line_number":239,"context_line":""},{"line_number":240,"context_line":"            # First class is this payload class, parent class is the 2nd"},{"line_number":241,"context_line":"            # one in the tuple"},{"line_number":242,"context_line":"            parent \u003d base_classes[1]"},{"line_number":243,"context_line":"            if (not hasattr(parent, \u0027SCHEMA\u0027) or"},{"line_number":244,"context_line":"                parent.SCHEMA !\u003d payload.SCHEMA):"},{"line_number":245,"context_line":"                payloads.append(payload)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a3c09a3_fcf1e701","line":242,"in_reply_to":"7a3c09a3_a4703da5","updated":"2017-01-18 15:37:18.000000000","message":"Maybe I\u0027m missing something.\n\n- NotificationPayloadBase doesn\u0027t have a .SCHEMA. So if CustomPayload does, I want it.\n- IF NotificationPayloadBase had a .SCHEMA and CustomPayload didn\u0027t, I wouldn\u0027t want it\n- IF NotificationPayloadBase had a .SCHEMA and CustomPayload had a different .SCHEMA, I would want it.\n\nIf we agree with the above -- are you saying that this method doesn\u0027t handle it? And if this method is that hard to understand/grok (since there is no test for testing this util unless we want to start adding tests for them), how can I make it easier to understand.","commit_id":"1e162bf622dd943b1c9a9ab2b94d3fb1498911d1"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"928743bc6c45fc657cc334a896be028f611974bd","unresolved":false,"context_lines":[{"line_number":239,"context_line":""},{"line_number":240,"context_line":"            # First class is this payload class, parent class is the 2nd"},{"line_number":241,"context_line":"            # one in the tuple"},{"line_number":242,"context_line":"            parent \u003d base_classes[1]"},{"line_number":243,"context_line":"            if (not hasattr(parent, \u0027SCHEMA\u0027) or"},{"line_number":244,"context_line":"                parent.SCHEMA !\u003d payload.SCHEMA):"},{"line_number":245,"context_line":"                payloads.append(payload)"}],"source_content_type":"text/x-python","patch_set":4,"id":"7a3c09a3_177e8c69","line":242,"in_reply_to":"7a3c09a3_fcf1e701","updated":"2017-01-18 15:47:24.000000000","message":"I think the logic is - class is inherited from NotificationPayloadBase AND its schema is different from the SCHEMA of the first object that has SCHEMA attr in mro.\n\nHere is what I\u0027m thinking of in terms of code:\n\n base_classes \u003d inspect.getmro(payload)\n if notification.NotificationPayloadBase not in base_classes:\n   # The class may have the desired name but it isn\u0027t a REAL\n   # Payload class; skip it.\n   continue\n for c in base_classes[1:]:\n   if hasattr(parent, \u0027SCHEMA\u0027) and parent.SCHEMA !\u003d payload.SCHEMA:\n     payloads.append(payload)\n     break","commit_id":"1e162bf622dd943b1c9a9ab2b94d3fb1498911d1"}]}