)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":30911,"name":"Jan Horstmann","email":"horstmann@osism.tech","username":"jhorstmann"},"change_message_id":"312526c8cff455dbe38b28f75bbca1e932c864b0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"c47b29b2_896fc7ec","updated":"2022-04-29 08:04:17.000000000","message":"This is probably not the best solution to this problem since it relies on a \"private\" method from openstacksdk.\nMaybe a better approach would be to expose the `details` flag in `cloud.list_machines()` in openstacksdk with a default to false.\nI shied away from that because I do not know anything about the compatibility guarantees made between openstacksdk versions and ansible-collections-openstack.\n","commit_id":"2acbf49681adb264249c6f899c445bcbc173d905"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"3c5208536a0b74d10675b795fee43f8a45593123","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"5d048a31_d9153765","in_reply_to":"c47b29b2_896fc7ec","updated":"2022-05-02 08:53:58.000000000","message":"Agree, we not want to use private methods from openstacksdk. But we probably have to refactor this module anyway for compatibility with the upcoming 1.0.0 release of the sdk. Since we have to break backward compatibility (due to changes in sdk) we can move from cloud.list_machines() to cloud.baremetal.nodes() and then ask for details\u003dTrue.\n\nFor our stable/1.0.0 branch we could relax our rules and allow to use this private method because it will only affect SDK 0.*.* releases which probably will be \"frozen\" after SDK 1.0.0 has been released.\n\nWe will need Sagi\u0027s etc. take on this.","commit_id":"2acbf49681adb264249c6f899c445bcbc173d905"},{"author":{"_account_id":30911,"name":"Jan Horstmann","email":"horstmann@osism.tech","username":"jhorstmann"},"change_message_id":"764c02ba7fc2bbda554293ab646fd15735ea99e7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"88793a96_8222e15c","updated":"2022-05-18 14:08:37.000000000","message":"I have changed this to not rely on any sdk internals. Any feedback is welcome :)","commit_id":"b7aaf49123fe302a9a7eb3eb3c19252c64b17dbb"},{"author":{"_account_id":10969,"name":"Shnaidman Sagi (Sergey)","display_name":"Shnaidman Sagi","email":"sshnaidm@redhat.com","username":"sergsh"},"change_message_id":"7803f157a02c1057bea726f16ad9cc7917d02f19","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"b72e2a70_aeb0cc80","updated":"2022-05-24 15:30:16.000000000","message":"Will let Jakub, Harald and others to review as well.","commit_id":"d448bd9e060fb9873fc52c3a0920b647ccfff93c"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"eaedfd124293a49152bae1fdf051f603c53cf3a8","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"26fe319f_92a59ed9","updated":"2022-05-31 10:44:57.000000000","message":"Thanks, Jan! Agree with your move from cloud layer functions to service proxies.\n\nThis allows us to get rid of Munch objects which then allows us to simplify/reduce the code even more. Let me upload a draft so show what i mean..","commit_id":"b50cd6c489b1e4fa504d258137a838e85c17cb4e"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"9a533b58e34b21cb0322dfb91f4549f483ee6df1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":6,"id":"73b1718a_8dec2511","updated":"2022-05-31 10:50:26.000000000","message":"The latest patchset ","commit_id":"8b8e8d44233fe4859969761509cc72150795d7f8"}],"plugins/modules/baremetal_node_info.py":[{"author":{"_account_id":10969,"name":"Shnaidman Sagi (Sergey)","display_name":"Shnaidman Sagi","email":"sshnaidm@redhat.com","username":"sergsh"},"change_message_id":"8f6009bed2c2f0bc74fdb07213ca5baf4c675548","unresolved":true,"context_lines":[{"line_number":540,"context_line":"        if machine:"},{"line_number":541,"context_line":"            machines.append(machine)"},{"line_number":542,"context_line":"        else:"},{"line_number":543,"context_line":"            machines \u003d list(filter(None,"},{"line_number":544,"context_line":"                                   [cloud.get_machine(machine.uuid)"},{"line_number":545,"context_line":"                                    for machine in cloud.list_machines()]))"},{"line_number":546,"context_line":""},{"line_number":547,"context_line":"        for machine in machines:"},{"line_number":548,"context_line":"            get_ports_and_portgroups(cloud, machine)"}],"source_content_type":"text/x-python","patch_set":2,"id":"af909156_0703a6e3","line":545,"range":{"start_line":543,"start_character":23,"end_line":545,"end_character":75},"updated":"2022-05-18 16:13:36.000000000","message":"Although I liked more the first approach, just the \"normalize\" function we could duplicate here instead of using internal methods.\nBut this is a good solution as well I think.","commit_id":"b7aaf49123fe302a9a7eb3eb3c19252c64b17dbb"},{"author":{"_account_id":30911,"name":"Jan Horstmann","email":"horstmann@osism.tech","username":"jhorstmann"},"change_message_id":"d46cdbe4a022a5240e074cc43329991a18792cdb","unresolved":false,"context_lines":[{"line_number":540,"context_line":"        if machine:"},{"line_number":541,"context_line":"            machines.append(machine)"},{"line_number":542,"context_line":"        else:"},{"line_number":543,"context_line":"            machines \u003d list(filter(None,"},{"line_number":544,"context_line":"                                   [cloud.get_machine(machine.uuid)"},{"line_number":545,"context_line":"                                    for machine in cloud.list_machines()]))"},{"line_number":546,"context_line":""},{"line_number":547,"context_line":"        for machine in machines:"},{"line_number":548,"context_line":"            get_ports_and_portgroups(cloud, machine)"}],"source_content_type":"text/x-python","patch_set":2,"id":"f08da5de_2dcd3d6b","line":545,"range":{"start_line":543,"start_character":23,"end_line":545,"end_character":75},"in_reply_to":"299c63a4_185dfc4d","updated":"2022-06-17 09:34:40.000000000","message":"Thanks for proposing a patch set to simplify this. I think that approach makes sense","commit_id":"b7aaf49123fe302a9a7eb3eb3c19252c64b17dbb"},{"author":{"_account_id":30911,"name":"Jan Horstmann","email":"horstmann@osism.tech","username":"jhorstmann"},"change_message_id":"aaa7269bb1786c58562aca51a886eddba629bc73","unresolved":true,"context_lines":[{"line_number":540,"context_line":"        if machine:"},{"line_number":541,"context_line":"            machines.append(machine)"},{"line_number":542,"context_line":"        else:"},{"line_number":543,"context_line":"            machines \u003d list(filter(None,"},{"line_number":544,"context_line":"                                   [cloud.get_machine(machine.uuid)"},{"line_number":545,"context_line":"                                    for machine in cloud.list_machines()]))"},{"line_number":546,"context_line":""},{"line_number":547,"context_line":"        for machine in machines:"},{"line_number":548,"context_line":"            get_ports_and_portgroups(cloud, machine)"}],"source_content_type":"text/x-python","patch_set":2,"id":"dc840ff5_c1076d1d","line":545,"range":{"start_line":543,"start_character":23,"end_line":545,"end_character":75},"in_reply_to":"a4188b79_dc5b6f24","updated":"2022-05-24 15:23:26.000000000","message":"I have now rewritten the module to get machines via the baremetal service proxy and convert them using the `to_dict()` method. The module relied on the machine object being `munch.Munch` before, but working with dict should not be a problem.\nLet me know what you think.","commit_id":"b7aaf49123fe302a9a7eb3eb3c19252c64b17dbb"},{"author":{"_account_id":30911,"name":"Jan Horstmann","email":"horstmann@osism.tech","username":"jhorstmann"},"change_message_id":"cad5a6ef3188d74bc16934a891c63c6a1bc0c6c8","unresolved":true,"context_lines":[{"line_number":540,"context_line":"        if machine:"},{"line_number":541,"context_line":"            machines.append(machine)"},{"line_number":542,"context_line":"        else:"},{"line_number":543,"context_line":"            machines \u003d list(filter(None,"},{"line_number":544,"context_line":"                                   [cloud.get_machine(machine.uuid)"},{"line_number":545,"context_line":"                                    for machine in cloud.list_machines()]))"},{"line_number":546,"context_line":""},{"line_number":547,"context_line":"        for machine in machines:"},{"line_number":548,"context_line":"            get_ports_and_portgroups(cloud, machine)"}],"source_content_type":"text/x-python","patch_set":2,"id":"a4188b79_dc5b6f24","line":545,"range":{"start_line":543,"start_character":23,"end_line":545,"end_character":75},"in_reply_to":"af909156_0703a6e3","updated":"2022-05-20 14:06:19.000000000","message":"The current solution will query the ironic API for every node listed, which is why I refrained from it in the first place. Using \"cloud.baremetal.nodes(details\u003dtrue)\" will just query the ironic API once.\n\nI could also do \n```\nmachines \u003d [machine._to_munch() for machine in cloud.baremetal.nodes(details\u003dTrue)]\n```\nand clean up the nova artifacts in `cleanup_node_properties()`.\nThat would also use a private method, but from the `machine` object.\n\nQuerying the API once is probably preferrable.","commit_id":"b7aaf49123fe302a9a7eb3eb3c19252c64b17dbb"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"93c9fa9f1f75e6a1897d3a095957bc8fb7b1c72b","unresolved":true,"context_lines":[{"line_number":540,"context_line":"        if machine:"},{"line_number":541,"context_line":"            machines.append(machine)"},{"line_number":542,"context_line":"        else:"},{"line_number":543,"context_line":"            machines \u003d list(filter(None,"},{"line_number":544,"context_line":"                                   [cloud.get_machine(machine.uuid)"},{"line_number":545,"context_line":"                                    for machine in cloud.list_machines()]))"},{"line_number":546,"context_line":""},{"line_number":547,"context_line":"        for machine in machines:"},{"line_number":548,"context_line":"            get_ports_and_portgroups(cloud, machine)"}],"source_content_type":"text/x-python","patch_set":2,"id":"299c63a4_185dfc4d","line":545,"range":{"start_line":543,"start_character":23,"end_line":545,"end_character":75},"in_reply_to":"dc840ff5_c1076d1d","updated":"2022-05-31 10:21:42.000000000","message":"_normalize_machine() will convert sdk resources into Munch objects [1]. This is certainly not what we want: We want to get away from Munch objects and use resource objects consistently across modules.\n\nJan\u0027s current approach of using service proxies directly is better. The cleanup can be improved or rather reduced though. Will upload an example later.\n\n\n[1] https://github.com/openstack/openstacksdk/blob/03cf733959abdac3202b623d7730ec7b09d5971a/openstack/cloud/_normalize.py#L1188","commit_id":"b7aaf49123fe302a9a7eb3eb3c19252c64b17dbb"},{"author":{"_account_id":10969,"name":"Shnaidman Sagi (Sergey)","display_name":"Shnaidman Sagi","email":"sshnaidm@redhat.com","username":"sergsh"},"change_message_id":"7803f157a02c1057bea726f16ad9cc7917d02f19","unresolved":true,"context_lines":[{"line_number":548,"context_line":"            module.fail_json(msg\u003d\u0027The baremetal node was not found\u0027)"},{"line_number":549,"context_line":""},{"line_number":550,"context_line":"        if machine:"},{"line_number":551,"context_line":"            machines.append(machine.to_dict(body\u003dTrue, headers\u003dFalse, original_names\u003dTrue))"},{"line_number":552,"context_line":"        else:"},{"line_number":553,"context_line":"            machines \u003d [machine.to_dict(body\u003dTrue, headers\u003dFalse, original_names\u003dTrue)"},{"line_number":554,"context_line":"                        for machine in cloud.baremetal.nodes(details\u003dTrue)]"}],"source_content_type":"text/x-python","patch_set":3,"id":"6b1ff36a_0b9676ca","line":551,"range":{"start_line":551,"start_character":70,"end_line":551,"end_character":89},"updated":"2022-05-24 15:30:16.000000000","message":"I think we can drop it for SDK\u003e\u003d1.0.0 (which is \"master\" version of collection) and leave it only for stable/1.0.0 branch. In total I think that\u0027s a great approach.","commit_id":"d448bd9e060fb9873fc52c3a0920b647ccfff93c"},{"author":{"_account_id":30911,"name":"Jan Horstmann","email":"horstmann@osism.tech","username":"jhorstmann"},"change_message_id":"653817d9881b54a9db312a6cfd2d21c1ac3b9793","unresolved":false,"context_lines":[{"line_number":548,"context_line":"            module.fail_json(msg\u003d\u0027The baremetal node was not found\u0027)"},{"line_number":549,"context_line":""},{"line_number":550,"context_line":"        if machine:"},{"line_number":551,"context_line":"            machines.append(machine.to_dict(body\u003dTrue, headers\u003dFalse, original_names\u003dTrue))"},{"line_number":552,"context_line":"        else:"},{"line_number":553,"context_line":"            machines \u003d [machine.to_dict(body\u003dTrue, headers\u003dFalse, original_names\u003dTrue)"},{"line_number":554,"context_line":"                        for machine in cloud.baremetal.nodes(details\u003dTrue)]"}],"source_content_type":"text/x-python","patch_set":3,"id":"ab3de7b1_a2dec078","line":551,"range":{"start_line":551,"start_character":70,"end_line":551,"end_character":89},"in_reply_to":"6b1ff36a_0b9676ca","updated":"2022-05-25 15:51:22.000000000","message":"Done","commit_id":"d448bd9e060fb9873fc52c3a0920b647ccfff93c"},{"author":{"_account_id":34208,"name":"Rafael Castillo","email":"rcastill@redhat.com","username":"rcastill"},"change_message_id":"01713d6627933a3e53daaad2818a348b3c1f7a32","unresolved":true,"context_lines":[{"line_number":537,"context_line":"    sdk, cloud \u003d openstack_cloud_from_module(module)"},{"line_number":538,"context_line":"    try:"},{"line_number":539,"context_line":"        if module.params[\u0027node\u0027]:"},{"line_number":540,"context_line":"            machine \u003d cloud.baremetal.get_node(module.params[\u0027node\u0027])"},{"line_number":541,"context_line":"        elif module.params[\u0027mac\u0027]:"},{"line_number":542,"context_line":"            nic \u003d cloud.get_nic_by_mac(module.params[\u0027mac\u0027])"},{"line_number":543,"context_line":"            if nic:"}],"source_content_type":"text/x-python","patch_set":4,"id":"045f122d_4ce56229","line":540,"updated":"2022-05-25 18:15:16.000000000","message":"find_node would be preferable to avoid an exception when the node isn\u0027t found.","commit_id":"435a831b4bb459d4deae13336d9ab9950612009f"},{"author":{"_account_id":30911,"name":"Jan Horstmann","email":"horstmann@osism.tech","username":"jhorstmann"},"change_message_id":"7ef957b87f46cda9ef7a6c4abf06174271b4144c","unresolved":false,"context_lines":[{"line_number":537,"context_line":"    sdk, cloud \u003d openstack_cloud_from_module(module)"},{"line_number":538,"context_line":"    try:"},{"line_number":539,"context_line":"        if module.params[\u0027node\u0027]:"},{"line_number":540,"context_line":"            machine \u003d cloud.baremetal.get_node(module.params[\u0027node\u0027])"},{"line_number":541,"context_line":"        elif module.params[\u0027mac\u0027]:"},{"line_number":542,"context_line":"            nic \u003d cloud.get_nic_by_mac(module.params[\u0027mac\u0027])"},{"line_number":543,"context_line":"            if nic:"}],"source_content_type":"text/x-python","patch_set":4,"id":"5063bffa_b2596854","line":540,"in_reply_to":"045f122d_4ce56229","updated":"2022-05-27 07:18:21.000000000","message":"Thanks for catching that","commit_id":"435a831b4bb459d4deae13336d9ab9950612009f"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"eaedfd124293a49152bae1fdf051f603c53cf3a8","unresolved":true,"context_lines":[{"line_number":486,"context_line":")"},{"line_number":487,"context_line":""},{"line_number":488,"context_line":""},{"line_number":489,"context_line":"def cleanup_node_properties(machine, cloud):"},{"line_number":490,"context_line":"    # states are links, not useful"},{"line_number":491,"context_line":"    machine.pop(\u0027states\u0027, None)"},{"line_number":492,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"dd331a3b_c3434c0f","line":489,"updated":"2022-05-31 10:44:57.000000000","message":"once we dropped all unnecessary pop statements, this function will only have two tasks: remove links and replace portgroup[\u0027ports\u0027] with results from cloud.baremetal.ports().\n\nThe latter makes more sense in get_ports_and_portgroups() and imho should be moved to that function. Hence we only have code to drop links left in this function. In all other modules we simply do not care about links and return them to the user. Because why should we care? We already did the api call and thus already got that info. So just return it. In worst case, the user will ignore it. So we can get rid of this cleanup_node_properties function completely. Less code without loosing functionality, less to maintain.","commit_id":"b50cd6c489b1e4fa504d258137a838e85c17cb4e"},{"author":{"_account_id":30911,"name":"Jan Horstmann","email":"horstmann@osism.tech","username":"jhorstmann"},"change_message_id":"d46cdbe4a022a5240e074cc43329991a18792cdb","unresolved":false,"context_lines":[{"line_number":486,"context_line":")"},{"line_number":487,"context_line":""},{"line_number":488,"context_line":""},{"line_number":489,"context_line":"def cleanup_node_properties(machine, cloud):"},{"line_number":490,"context_line":"    # states are links, not useful"},{"line_number":491,"context_line":"    machine.pop(\u0027states\u0027, None)"},{"line_number":492,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"abf5900d_55df47ca","line":489,"in_reply_to":"dd331a3b_c3434c0f","updated":"2022-06-17 09:34:40.000000000","message":"That makes sense","commit_id":"b50cd6c489b1e4fa504d258137a838e85c17cb4e"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"eaedfd124293a49152bae1fdf051f603c53cf3a8","unresolved":true,"context_lines":[{"line_number":496,"context_line":"    machine.pop(\u0027HUMAN_ID\u0027, None)"},{"line_number":497,"context_line":"    machine.pop(\u0027human_id\u0027, None)"},{"line_number":498,"context_line":"    machine.pop(\u0027request_ids\u0027, None)"},{"line_number":499,"context_line":"    machine.pop(\u0027x_openstack_request_ids\u0027, None)"},{"line_number":500,"context_line":""},{"line_number":501,"context_line":"    for port in machine[\u0027ports\u0027]:"},{"line_number":502,"context_line":"        # links are not useful"}],"source_content_type":"text/x-python","patch_set":5,"id":"411653b3_056ea179","line":499,"updated":"2022-05-31 10:44:57.000000000","message":"These pop statements (copied from [1]) can be removed because we do not use SDK\u0027s cloud layer and thus no _normalize_machine and thus should only have attributes which are listed here [2].\n\n[1] https://github.com/openstack/openstacksdk/blob/03cf733959abdac3202b623d7730ec7b09d5971a/openstack/cloud/_normalize.py#L170\n[2] https://github.com/openstack/openstacksdk/blob/master/openstack/baremetal/v1/node.py","commit_id":"b50cd6c489b1e4fa504d258137a838e85c17cb4e"},{"author":{"_account_id":30911,"name":"Jan Horstmann","email":"horstmann@osism.tech","username":"jhorstmann"},"change_message_id":"d46cdbe4a022a5240e074cc43329991a18792cdb","unresolved":false,"context_lines":[{"line_number":496,"context_line":"    machine.pop(\u0027HUMAN_ID\u0027, None)"},{"line_number":497,"context_line":"    machine.pop(\u0027human_id\u0027, None)"},{"line_number":498,"context_line":"    machine.pop(\u0027request_ids\u0027, None)"},{"line_number":499,"context_line":"    machine.pop(\u0027x_openstack_request_ids\u0027, None)"},{"line_number":500,"context_line":""},{"line_number":501,"context_line":"    for port in machine[\u0027ports\u0027]:"},{"line_number":502,"context_line":"        # links are not useful"}],"source_content_type":"text/x-python","patch_set":5,"id":"c21eb4ff_04ebceab","line":499,"in_reply_to":"411653b3_056ea179","updated":"2022-06-17 09:34:40.000000000","message":"I have to admit that I blindly copied those without checking where they are generated. Thanks for pointing this out","commit_id":"b50cd6c489b1e4fa504d258137a838e85c17cb4e"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"eaedfd124293a49152bae1fdf051f603c53cf3a8","unresolved":true,"context_lines":[{"line_number":502,"context_line":"        # links are not useful"},{"line_number":503,"context_line":"        port.pop(\u0027links\u0027, None)"},{"line_number":504,"context_line":"        # redundant, location is in on machine as well"},{"line_number":505,"context_line":"        port.pop(\u0027location\u0027, None)"},{"line_number":506,"context_line":""},{"line_number":507,"context_line":"    for portgroup in machine[\u0027portgroups\u0027]:"},{"line_number":508,"context_line":"        # links are not useful"}],"source_content_type":"text/x-python","patch_set":5,"id":"5078f684_f59eb6b7","line":505,"updated":"2022-05-31 10:44:57.000000000","message":"SDK\u0027s Resource.to_dict(computed\u003dFalse) will drop \u0027location\u0027 fields.","commit_id":"b50cd6c489b1e4fa504d258137a838e85c17cb4e"},{"author":{"_account_id":30911,"name":"Jan Horstmann","email":"horstmann@osism.tech","username":"jhorstmann"},"change_message_id":"d46cdbe4a022a5240e074cc43329991a18792cdb","unresolved":false,"context_lines":[{"line_number":502,"context_line":"        # links are not useful"},{"line_number":503,"context_line":"        port.pop(\u0027links\u0027, None)"},{"line_number":504,"context_line":"        # redundant, location is in on machine as well"},{"line_number":505,"context_line":"        port.pop(\u0027location\u0027, None)"},{"line_number":506,"context_line":""},{"line_number":507,"context_line":"    for portgroup in machine[\u0027portgroups\u0027]:"},{"line_number":508,"context_line":"        # links are not useful"}],"source_content_type":"text/x-python","patch_set":5,"id":"f6ffaa84_46f9df6f","line":505,"in_reply_to":"5078f684_f59eb6b7","updated":"2022-06-17 09:34:40.000000000","message":"Thank you for that hint","commit_id":"b50cd6c489b1e4fa504d258137a838e85c17cb4e"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"eaedfd124293a49152bae1fdf051f603c53cf3a8","unresolved":true,"context_lines":[{"line_number":508,"context_line":"        # links are not useful"},{"line_number":509,"context_line":"        portgroup.pop(\u0027links\u0027, None)"},{"line_number":510,"context_line":"        # redundant, location is in on machine as well"},{"line_number":511,"context_line":"        portgroup.pop(\u0027location\u0027, None)"},{"line_number":512,"context_line":"        # links to ports are not useful, replace with list of port uuid\u0027s"},{"line_number":513,"context_line":"        portgroup[\u0027ports\u0027] \u003d [x.id for x in list("},{"line_number":514,"context_line":"            cloud.baremetal.ports(portgroup\u003dportgroup[\u0027id\u0027]))]"}],"source_content_type":"text/x-python","patch_set":5,"id":"9e10f04f_004ade77","line":511,"updated":"2022-05-31 10:44:57.000000000","message":"ditto.","commit_id":"b50cd6c489b1e4fa504d258137a838e85c17cb4e"},{"author":{"_account_id":30911,"name":"Jan Horstmann","email":"horstmann@osism.tech","username":"jhorstmann"},"change_message_id":"d46cdbe4a022a5240e074cc43329991a18792cdb","unresolved":false,"context_lines":[{"line_number":508,"context_line":"        # links are not useful"},{"line_number":509,"context_line":"        portgroup.pop(\u0027links\u0027, None)"},{"line_number":510,"context_line":"        # redundant, location is in on machine as well"},{"line_number":511,"context_line":"        portgroup.pop(\u0027location\u0027, None)"},{"line_number":512,"context_line":"        # links to ports are not useful, replace with list of port uuid\u0027s"},{"line_number":513,"context_line":"        portgroup[\u0027ports\u0027] \u003d [x.id for x in list("},{"line_number":514,"context_line":"            cloud.baremetal.ports(portgroup\u003dportgroup[\u0027id\u0027]))]"}],"source_content_type":"text/x-python","patch_set":5,"id":"f1c37bb1_71795eaa","line":511,"in_reply_to":"9e10f04f_004ade77","updated":"2022-06-17 09:34:40.000000000","message":"Done","commit_id":"b50cd6c489b1e4fa504d258137a838e85c17cb4e"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"eaedfd124293a49152bae1fdf051f603c53cf3a8","unresolved":true,"context_lines":[{"line_number":510,"context_line":"        # redundant, location is in on machine as well"},{"line_number":511,"context_line":"        portgroup.pop(\u0027location\u0027, None)"},{"line_number":512,"context_line":"        # links to ports are not useful, replace with list of port uuid\u0027s"},{"line_number":513,"context_line":"        portgroup[\u0027ports\u0027] \u003d [x.id for x in list("},{"line_number":514,"context_line":"            cloud.baremetal.ports(portgroup\u003dportgroup[\u0027id\u0027]))]"},{"line_number":515,"context_line":""},{"line_number":516,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"0c107764_da4ace04","line":513,"updated":"2022-05-31 10:44:57.000000000","message":"Why drop all info about a port except \"id\"? We already did an api call and have all info about ports so we could just return the whole port.","commit_id":"b50cd6c489b1e4fa504d258137a838e85c17cb4e"},{"author":{"_account_id":30911,"name":"Jan Horstmann","email":"horstmann@osism.tech","username":"jhorstmann"},"change_message_id":"d46cdbe4a022a5240e074cc43329991a18792cdb","unresolved":false,"context_lines":[{"line_number":510,"context_line":"        # redundant, location is in on machine as well"},{"line_number":511,"context_line":"        portgroup.pop(\u0027location\u0027, None)"},{"line_number":512,"context_line":"        # links to ports are not useful, replace with list of port uuid\u0027s"},{"line_number":513,"context_line":"        portgroup[\u0027ports\u0027] \u003d [x.id for x in list("},{"line_number":514,"context_line":"            cloud.baremetal.ports(portgroup\u003dportgroup[\u0027id\u0027]))]"},{"line_number":515,"context_line":""},{"line_number":516,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"b656f36b_a88e9f74","line":513,"in_reply_to":"0c107764_da4ace04","updated":"2022-06-17 09:34:40.000000000","message":"Sounds reasonable","commit_id":"b50cd6c489b1e4fa504d258137a838e85c17cb4e"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"9a533b58e34b21cb0322dfb91f4549f483ee6df1","unresolved":true,"context_lines":[{"line_number":57,"context_line":"                 provider."},{"line_number":58,"context_line":"    returned: always, but can be null"},{"line_number":59,"context_line":"    type: complex"},{"line_number":60,"context_line":"    contains:"},{"line_number":61,"context_line":"        allocation_uuid:"},{"line_number":62,"context_line":"            description: The UUID of the allocation associated with the node."},{"line_number":63,"context_line":"                         If not null, will be the same as instance_uuid (the"}],"source_content_type":"text/x-python","patch_set":6,"id":"654e76b0_3ffc4148","line":60,"updated":"2022-05-31 10:50:26.000000000","message":"Since we moved from Munch objects to SDK\u0027s resource objects, we will have to update this documentation as well. One can get doc updates from here:\n\nhttps://github.com/openstack/openstacksdk/blob/master/openstack/baremetal/v1/node.py\nhttps://github.com/openstack/openstacksdk/blob/master/openstack/baremetal/v1/port.py\nhttps://github.com/openstack/openstacksdk/blob/master/openstack/baremetal/v1/port_group.py","commit_id":"8b8e8d44233fe4859969761509cc72150795d7f8"},{"author":{"_account_id":30911,"name":"Jan Horstmann","email":"horstmann@osism.tech","username":"jhorstmann"},"change_message_id":"d46cdbe4a022a5240e074cc43329991a18792cdb","unresolved":false,"context_lines":[{"line_number":57,"context_line":"                 provider."},{"line_number":58,"context_line":"    returned: always, but can be null"},{"line_number":59,"context_line":"    type: complex"},{"line_number":60,"context_line":"    contains:"},{"line_number":61,"context_line":"        allocation_uuid:"},{"line_number":62,"context_line":"            description: The UUID of the allocation associated with the node."},{"line_number":63,"context_line":"                         If not null, will be the same as instance_uuid (the"}],"source_content_type":"text/x-python","patch_set":6,"id":"0b0e295f_9716a710","line":60,"in_reply_to":"654e76b0_3ffc4148","updated":"2022-06-17 09:34:40.000000000","message":"I think I have made all necessary changes","commit_id":"8b8e8d44233fe4859969761509cc72150795d7f8"},{"author":{"_account_id":30911,"name":"Jan Horstmann","email":"horstmann@osism.tech","username":"jhorstmann"},"change_message_id":"d46cdbe4a022a5240e074cc43329991a18792cdb","unresolved":true,"context_lines":[{"line_number":560,"context_line":"def get_ports_and_portgroups(cloud, machine):"},{"line_number":561,"context_line":"    machine[\u0027ports\u0027] \u003d [nic.to_dict(computed\u003dFalse)"},{"line_number":562,"context_line":"                        for nic in cloud.baremetal.ports("},{"line_number":563,"context_line":"                            details\u003dTrue, node_id\u003dmachine[\u0027id\u0027])]"},{"line_number":564,"context_line":""},{"line_number":565,"context_line":"    machine[\u0027port_groups\u0027] \u003d [grp.to_dict(computed\u003dFalse) for grp in"},{"line_number":566,"context_line":"                              cloud.baremetal.port_groups(node\u003dmachine[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":8,"id":"e56b5db7_d646a054","line":563,"updated":"2022-06-17 09:34:40.000000000","message":"`uuid` was changed to `id` when switching to the service proxy. I missed that before","commit_id":"4c120cb73d8fbddcbcd7db26d32fdbe946cd4e64"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"4d7018efb3576859e949a3da41761c407e801df6","unresolved":false,"context_lines":[{"line_number":560,"context_line":"def get_ports_and_portgroups(cloud, machine):"},{"line_number":561,"context_line":"    machine[\u0027ports\u0027] \u003d [nic.to_dict(computed\u003dFalse)"},{"line_number":562,"context_line":"                        for nic in cloud.baremetal.ports("},{"line_number":563,"context_line":"                            details\u003dTrue, node_id\u003dmachine[\u0027id\u0027])]"},{"line_number":564,"context_line":""},{"line_number":565,"context_line":"    machine[\u0027port_groups\u0027] \u003d [grp.to_dict(computed\u003dFalse) for grp in"},{"line_number":566,"context_line":"                              cloud.baremetal.port_groups(node\u003dmachine[\u0027id\u0027],"}],"source_content_type":"text/x-python","patch_set":8,"id":"9b83a907_9040ea1f","line":563,"in_reply_to":"e56b5db7_d646a054","updated":"2022-06-23 10:56:23.000000000","message":"Good catch!","commit_id":"4c120cb73d8fbddcbcd7db26d32fdbe946cd4e64"},{"author":{"_account_id":30911,"name":"Jan Horstmann","email":"horstmann@osism.tech","username":"jhorstmann"},"change_message_id":"d46cdbe4a022a5240e074cc43329991a18792cdb","unresolved":true,"context_lines":[{"line_number":568,"context_line":""},{"line_number":569,"context_line":"    # links to ports are not useful, replace with list of ports"},{"line_number":570,"context_line":"    for port_group in machine[\u0027port_groups\u0027]:"},{"line_number":571,"context_line":"        port_group[\u0027ports\u0027] \u003d [port for port in machine[\u0027ports\u0027]"},{"line_number":572,"context_line":"                               if port[\u0027port_group_id\u0027] \u003d\u003d port_group[\u0027id\u0027]]"},{"line_number":573,"context_line":""},{"line_number":574,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"bb2ea16b_bbc93a0e","line":571,"updated":"2022-06-17 09:34:40.000000000","message":"Ironic does not allow ports belonging to another node in a node\u0027s port group ([1]). Therefore, I think, we may safely skip this API call and just iterate through the ports we already have for the machine.\nWhat do you think?\n\n\n[1]\nhttps://opendev.org/openstack/ironic/src/commit/0659485d630b8651faa633f98b1802bdf244f186/ironic/api/controllers/v1/port.py#L547","commit_id":"4c120cb73d8fbddcbcd7db26d32fdbe946cd4e64"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"4d7018efb3576859e949a3da41761c407e801df6","unresolved":false,"context_lines":[{"line_number":568,"context_line":""},{"line_number":569,"context_line":"    # links to ports are not useful, replace with list of ports"},{"line_number":570,"context_line":"    for port_group in machine[\u0027port_groups\u0027]:"},{"line_number":571,"context_line":"        port_group[\u0027ports\u0027] \u003d [port for port in machine[\u0027ports\u0027]"},{"line_number":572,"context_line":"                               if port[\u0027port_group_id\u0027] \u003d\u003d port_group[\u0027id\u0027]]"},{"line_number":573,"context_line":""},{"line_number":574,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"e474b3b4_ec022a48","line":571,"in_reply_to":"bb2ea16b_bbc93a0e","updated":"2022-06-23 10:56:23.000000000","message":"Oh, o/c good idea ^^","commit_id":"4c120cb73d8fbddcbcd7db26d32fdbe946cd4e64"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"4d7018efb3576859e949a3da41761c407e801df6","unresolved":false,"context_lines":[{"line_number":590,"context_line":"        if module.params[\u0027node\u0027]:"},{"line_number":591,"context_line":"            machine \u003d cloud.baremetal.find_node(module.params[\u0027node\u0027])"},{"line_number":592,"context_line":"        elif module.params[\u0027mac\u0027]:"},{"line_number":593,"context_line":"            nic \u003d next(cloud.baremetal.ports(address\u003dmodule.params[\u0027mac\u0027],"},{"line_number":594,"context_line":"                                             fields\u003d[\u0027node_id\u0027]), None)"},{"line_number":595,"context_line":"            if nic:"},{"line_number":596,"context_line":"                machine \u003d cloud.baremetal.find_node(nic[\u0027node_id\u0027])"}],"source_content_type":"text/x-python","patch_set":9,"id":"9b02baa4_f4ef772b","line":593,"updated":"2022-06-23 10:56:23.000000000","message":"Nice, this is (probably?) more efficient than get_nic_by_mac ☺️","commit_id":"9b62cd7734e7f3233372eecfb101d5cb6f9566f4"}]}