)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"7af1de275149fd99acd172ae042f89d8a127cb24","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":9,"id":"26c35e65_3f55081a","updated":"2022-07-06 14:33:25.000000000","message":"Latest patchset is rebased on latest routers_info patch.","commit_id":"d87089a1e9f149fed57139ed811846eabfdfb9ac"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"63ec8dcadb29e957ff4016403f4a28e024fd16de","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"bae0c60b_3bc282cb","updated":"2022-07-12 12:13:00.000000000","message":"Applied minor, mostly cosmetic changes to your patch, Rafael:\n\n* changed module result type from complex to dict because we no longer return Munch objects\n* expanded and fixed module results docs\n* moved ext_ips_spec into the class because global scope is not necessary\n* sorted argument_spec and attribute docs by attribute name and fixed indentation of the latter\n* limited line length to 80 chars to please local linter. i prefer 120 chars or more but let us stick to 80 chars to be consistent with other openstack projects","commit_id":"ff92f13dbbb3f0899ac467171ca34f8ffd870989"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"5340ba0c21de05b9b08412aad36a42630c158ceb","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"8699ef6c_ed1c466b","updated":"2022-07-14 08:15:18.000000000","message":"After having read [1] it feels odd to update your patch for cosmetic and other minor changes. I will revert my changes and submit a follow up patch for non-important changes.\n\n[1] http://repohealth.io/blog/code-review-how-to-make-enemies/","commit_id":"daa9a69209f02629e641158e4a31e9d5db7cd914"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"aaff34f7baef40d64f2315db6e240df56ec7a855","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"b31e7525_52969512","updated":"2022-07-12 13:01:26.000000000","message":"What the latest two patchsets do:\n* dropped self.fail() calls and let openstacksdk handle missing networks, subnets.. instead. Less code, less to maintain 😉\n* do not check whether self.conn.network.update_router() returned a None value because this is not possible with our input values. actually i cannot find any code path in openstacksdk 0.99.0 where this function could return a None value.\n* no need to check if router is None before returning it because cannot be None except if openstacksdk has a bug\n* replaced self.conn.delete_router with self.conn.network.delete_router because we already have a router object\n* replaced self.conn.remove_router_interface with self.conn.network.remove_interface_from_router because the latter simply calls the former anyway\n* limited more lines to 80 chars which my previous patchset missed","commit_id":"daa9a69209f02629e641158e4a31e9d5db7cd914"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"b165b7feb2c3b21a3d7fc01b03365320808d80ea","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"19394004_3da80212","in_reply_to":"8699ef6c_ed1c466b","updated":"2022-07-14 08:30:01.000000000","message":"Follow up patch with all the refactorings for easier reviewing of this patch:\n\n  https://review.opendev.org/c/openstack/ansible-collections-openstack/+/849793","commit_id":"daa9a69209f02629e641158e4a31e9d5db7cd914"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"b710f3e3eb2b1c076b09d658081ee944ac27aae9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"30651098_8683437f","updated":"2022-07-15 08:37:25.000000000","message":"Excellent refactoring, Rafael! Especially the router update part is much better to understand now ☺️","commit_id":"1b071aca6f1ff1a67962d7c0718957d05a391562"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"f71a4370727520d390b4c51691cf48ff5312fd43","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"94dcc0a2_54315a0d","updated":"2022-07-14 08:16:43.000000000","message":"Latest patchset 14 is the same as patchset 10 but rebased on top of master branch.","commit_id":"1b071aca6f1ff1a67962d7c0718957d05a391562"},{"author":{"_account_id":34208,"name":"Rafael Castillo","email":"rcastill@redhat.com","username":"rcastill"},"change_message_id":"bb6682adb6c455519deff8526a19713d5e8886f3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":18,"id":"83bf61df_ae36ccb3","updated":"2022-07-22 05:29:42.000000000","message":"recheck","commit_id":"a8ff316cecbcba830c5ce4dcbcbf0033235a52c6"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"014457a54346af591243b4203a55e530e254042d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":27,"id":"ddc50066_aff85dad","updated":"2022-07-28 14:48:08.000000000","message":"recheck Zuul, you can either let our jobs pass or i will ask again. Your choice.","commit_id":"3fdbd56a585f4079ef348ad7060fd9836e85e578"}],"ci/roles/router/defaults/main.yml":[{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"75817aa985d3f53f7a7e3a2f83b3a0e774747bc1","unresolved":true,"context_lines":[{"line_number":20,"context_line":"external_network_name: ansible_external_net"},{"line_number":21,"context_line":"network_external: true"},{"line_number":22,"context_line":"router_name: ansible_router"},{"line_number":23,"context_line":"test_subnets:"},{"line_number":24,"context_line":"  - cloud: \"{{ cloud }}\""},{"line_number":25,"context_line":"    state: present"},{"line_number":26,"context_line":"    network_name: \"{{ network_name }}\""}],"source_content_type":"text/x-yaml","patch_set":20,"id":"96604a81_f9a5fb82","line":23,"updated":"2022-07-25 12:34:55.000000000","message":"Please define network_name in this role to avoid undefined behaviour (kind of..).","commit_id":"02fb83674ac625c53947296a590d31da0591787a"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"484b4e0d68589504d32e7d9c3036299120c9b310","unresolved":false,"context_lines":[{"line_number":20,"context_line":"external_network_name: ansible_external_net"},{"line_number":21,"context_line":"network_external: true"},{"line_number":22,"context_line":"router_name: ansible_router"},{"line_number":23,"context_line":"test_subnets:"},{"line_number":24,"context_line":"  - cloud: \"{{ cloud }}\""},{"line_number":25,"context_line":"    state: present"},{"line_number":26,"context_line":"    network_name: \"{{ network_name }}\""}],"source_content_type":"text/x-yaml","patch_set":20,"id":"7f9ce083_a44908c3","line":23,"in_reply_to":"96604a81_f9a5fb82","updated":"2022-07-28 08:23:17.000000000","message":"Done","commit_id":"02fb83674ac625c53947296a590d31da0591787a"}],"ci/roles/router/tasks/main.yml":[{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"dc163adb99c1508f34c902e0f7ef42d394d6e302","unresolved":true,"context_lines":[{"line_number":58,"context_line":"     state: present"},{"line_number":59,"context_line":"     name: \"{{ router_name }}\""},{"line_number":60,"context_line":"     interfaces:"},{"line_number":61,"context_line":"         - shade_subnet1"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"- name: Update router (add interface) again"},{"line_number":64,"context_line":"  openstack.cloud.router:"}],"source_content_type":"text/x-yaml","patch_set":20,"id":"9d1c708c_99d9d507","line":61,"updated":"2022-07-25 09:33:37.000000000","message":"This is supposed to add an interface to shade_subnet1. The log [1] reports that the router has been changed but the interface is not returned:\n\n  changed: [localhost] \u003d\u003e {\n      \"changed\": true,\n      \"invocation\": {\n          \"module_args\": {\n              \"api_timeout\": null,\n              \"auth\": null,\n              \"auth_type\": null,\n              \"availability_zone\": null,\n              \"ca_cert\": null,\n              \"client_cert\": null,\n              \"client_key\": null,\n              \"enable_snat\": null,\n              \"external_fixed_ips\": null,\n              \"external_gateway_info\": null,\n              \"interface\": \"public\",\n              \"interfaces\": [\n                  \"shade_subnet1\"\n              ],\n              \"is_admin_state_up\": true,\n              \"name\": \"ansible_router\",\n              \"network\": null,\n              \"project\": null,\n              \"region_name\": null,\n              \"sdk_log_level\": \"INFO\",\n              \"sdk_log_path\": null,\n              \"state\": \"present\",\n              \"timeout\": 180,\n              \"validate_certs\": null,\n              \"wait\": true\n          }\n      },\n      \"router\": {\n          \"availability_zone_hints\": [],\n          \"availability_zones\": [],\n          \"created_at\": \"2022-07-22T15:23:15Z\",\n          \"description\": \"\",\n          \"enable_ndp_proxy\": null,\n          \"external_gateway_info\": null,\n          \"flavor_id\": null,\n          \"id\": \"82180dff-550d-4d1b-ba2b-0e64185ceeb1\",\n          \"is_admin_state_up\": true,\n          \"is_distributed\": false,\n          \"is_ha\": false,\n          \"name\": \"ansible_router\",\n          \"project_id\": \"31a080f26dcc46c5b01ea9e7dd1193c8\",\n          \"revision_number\": 1,\n          \"routes\": [],\n          \"status\": \"ACTIVE\",\n          \"tags\": [],\n          \"tenant_id\": \"31a080f26dcc46c5b01ea9e7dd1193c8\",\n          \"updated_at\": \"2022-07-22T15:23:15Z\"\n      }\n  }\n\nPlease add assertions on the returned interfaces here, please.\n\n[1] https://storage.bhs.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_4ae/846446/20/check/ansible-collections-openstack-functional-devstack/4aef592/job-output.txt","commit_id":"02fb83674ac625c53947296a590d31da0591787a"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"a09668d4a901a8b1cd8fb81f8260b2f8f35a1565","unresolved":false,"context_lines":[{"line_number":58,"context_line":"     state: present"},{"line_number":59,"context_line":"     name: \"{{ router_name }}\""},{"line_number":60,"context_line":"     interfaces:"},{"line_number":61,"context_line":"         - shade_subnet1"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"- name: Update router (add interface) again"},{"line_number":64,"context_line":"  openstack.cloud.router:"}],"source_content_type":"text/x-yaml","patch_set":20,"id":"ef0781b9_3883b394","line":61,"in_reply_to":"24a46f6c_072842b5","updated":"2022-07-25 10:50:45.000000000","message":"No need to add assertions because i added them myself some time ago, see below 🤦","commit_id":"02fb83674ac625c53947296a590d31da0591787a"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"b5f9be1932d3dd13c7d2861ec4da8bee7aae11b3","unresolved":true,"context_lines":[{"line_number":58,"context_line":"     state: present"},{"line_number":59,"context_line":"     name: \"{{ router_name }}\""},{"line_number":60,"context_line":"     interfaces:"},{"line_number":61,"context_line":"         - shade_subnet1"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":"- name: Update router (add interface) again"},{"line_number":64,"context_line":"  openstack.cloud.router:"}],"source_content_type":"text/x-yaml","patch_set":20,"id":"24a46f6c_072842b5","line":61,"in_reply_to":"9d1c708c_99d9d507","updated":"2022-07-25 09:53:04.000000000","message":"omg, router does not return interfaces_info because it is a custom hack [1] in routers_info module. We should get rid of this interfaces_info field in routers_info...\n\nBut please add assertions anyway, please.\n\n[1] https://opendev.org/openstack/ansible-collections-openstack/src/commit/2d60a045a68a47cf1dd13c61493809dcb8c5aedd/plugins/modules/routers_info.py#L209","commit_id":"02fb83674ac625c53947296a590d31da0591787a"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"5487e4a81a801748eafe720806e7fbca1cf2df6b","unresolved":true,"context_lines":[{"line_number":480,"context_line":""},{"line_number":481,"context_line":"- name: Verify routers info"},{"line_number":482,"context_line":"  assert:"},{"line_number":483,"context_line":"    that: result | community.general.json_query(ip_address_query)"},{"line_number":484,"context_line":"  vars:"},{"line_number":485,"context_line":"    ip_address_query: \"routers[0].interfaces_info[?ip_address\u003d\u003d\u002710.7.7.42\u0027]\""},{"line_number":486,"context_line":""}],"source_content_type":"text/x-yaml","patch_set":20,"id":"64c7a0ae_5c2633b9","line":483,"updated":"2022-07-25 13:04:59.000000000","message":"This requires the community.general collection. How about replacing this with e.g.:\n\n  that: \"\u002710.7.7.42\u0027 in result.routers[0].interfaces_info|map(attribute\u003d\u0027ip_address\u0027)\"","commit_id":"02fb83674ac625c53947296a590d31da0591787a"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"7d47ffd3b337117addf787c17edef010fbe50867","unresolved":false,"context_lines":[{"line_number":480,"context_line":""},{"line_number":481,"context_line":"- name: Verify routers info"},{"line_number":482,"context_line":"  assert:"},{"line_number":483,"context_line":"    that: result | community.general.json_query(ip_address_query)"},{"line_number":484,"context_line":"  vars:"},{"line_number":485,"context_line":"    ip_address_query: \"routers[0].interfaces_info[?ip_address\u003d\u003d\u002710.7.7.42\u0027]\""},{"line_number":486,"context_line":""}],"source_content_type":"text/x-yaml","patch_set":20,"id":"b8f26e3c_597072cd","line":483,"in_reply_to":"64c7a0ae_5c2633b9","updated":"2022-07-28 08:22:51.000000000","message":"Done","commit_id":"02fb83674ac625c53947296a590d31da0591787a"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"0c3cd96c23ad3c5717dce4c1ee3182e690c5915d","unresolved":true,"context_lines":[{"line_number":516,"context_line":"    cloud: \"{{ cloud }}\""},{"line_number":517,"context_line":"    name: \"{{ item.id }}\""},{"line_number":518,"context_line":"    state: absent"},{"line_number":519,"context_line":"  loop: \"{{ existing_ports.ports }}\""},{"line_number":520,"context_line":""},{"line_number":521,"context_line":"- name: Delete subnet5"},{"line_number":522,"context_line":"  openstack.cloud.subnet:"}],"source_content_type":"text/x-yaml","patch_set":20,"id":"74c1728d_32058b01","line":519,"updated":"2022-07-25 13:34:38.000000000","message":"Please only remove what has been created by this role. If there is still a port left, then deletion of the subnet should fail and we want to see that because this could indicate a bug in our code.","commit_id":"02fb83674ac625c53947296a590d31da0591787a"}],"plugins/modules/router.py":[{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"b710f3e3eb2b1c076b09d658081ee944ac27aae9","unresolved":true,"context_lines":[{"line_number":295,"context_line":"    def _needs_update(self, router, kwargs, external_fixed_ips, to_add, to_remove,"},{"line_number":296,"context_line":"                      missing_port_ids):"},{"line_number":297,"context_line":"        \"\"\"Decide if the given router needs an update.\"\"\""},{"line_number":298,"context_line":"        self.reason \u003d \u0027nope\u0027"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"        cur_ext_gw_info \u003d router[\u0027external_gateway_info\u0027]"},{"line_number":301,"context_line":"        if \u0027external_gateway_info\u0027 in kwargs:"}],"source_content_type":"text/x-python","patch_set":14,"id":"8fb80023_4952b899","line":298,"updated":"2022-07-15 08:37:25.000000000","message":"Missing?\n\n    if router[\u0027is_admin_state_up\u0027] !\u003d self.params[\u0027is_admin_state_up\u0027]:\n        return True","commit_id":"1b071aca6f1ff1a67962d7c0718957d05a391562"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"b710f3e3eb2b1c076b09d658081ee944ac27aae9","unresolved":true,"context_lines":[{"line_number":295,"context_line":"    def _needs_update(self, router, kwargs, external_fixed_ips, to_add, to_remove,"},{"line_number":296,"context_line":"                      missing_port_ids):"},{"line_number":297,"context_line":"        \"\"\"Decide if the given router needs an update.\"\"\""},{"line_number":298,"context_line":"        self.reason \u003d \u0027nope\u0027"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"        cur_ext_gw_info \u003d router[\u0027external_gateway_info\u0027]"},{"line_number":301,"context_line":"        if \u0027external_gateway_info\u0027 in kwargs:"}],"source_content_type":"text/x-python","patch_set":14,"id":"17eaf5b7_12ba9e48","line":298,"updated":"2022-07-15 08:37:25.000000000","message":"self.reason helps with debugging during development, but its not a reliable indicator to the user since it is only returned if it is actually updated. I would turn all self.reason statements into code comments. What do you think?","commit_id":"1b071aca6f1ff1a67962d7c0718957d05a391562"},{"author":{"_account_id":34208,"name":"Rafael Castillo","email":"rcastill@redhat.com","username":"rcastill"},"change_message_id":"661ca31501bdf8f75c664b9ba4554f987b69030e","unresolved":false,"context_lines":[{"line_number":295,"context_line":"    def _needs_update(self, router, kwargs, external_fixed_ips, to_add, to_remove,"},{"line_number":296,"context_line":"                      missing_port_ids):"},{"line_number":297,"context_line":"        \"\"\"Decide if the given router needs an update.\"\"\""},{"line_number":298,"context_line":"        self.reason \u003d \u0027nope\u0027"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"        cur_ext_gw_info \u003d router[\u0027external_gateway_info\u0027]"},{"line_number":301,"context_line":"        if \u0027external_gateway_info\u0027 in kwargs:"}],"source_content_type":"text/x-python","patch_set":14,"id":"240bd5f1_7b047ba1","line":298,"in_reply_to":"17eaf5b7_12ba9e48","updated":"2022-07-15 17:50:03.000000000","message":"was meant to remove that. My bad","commit_id":"1b071aca6f1ff1a67962d7c0718957d05a391562"},{"author":{"_account_id":34208,"name":"Rafael Castillo","email":"rcastill@redhat.com","username":"rcastill"},"change_message_id":"661ca31501bdf8f75c664b9ba4554f987b69030e","unresolved":true,"context_lines":[{"line_number":295,"context_line":"    def _needs_update(self, router, kwargs, external_fixed_ips, to_add, to_remove,"},{"line_number":296,"context_line":"                      missing_port_ids):"},{"line_number":297,"context_line":"        \"\"\"Decide if the given router needs an update.\"\"\""},{"line_number":298,"context_line":"        self.reason \u003d \u0027nope\u0027"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"        cur_ext_gw_info \u003d router[\u0027external_gateway_info\u0027]"},{"line_number":301,"context_line":"        if \u0027external_gateway_info\u0027 in kwargs:"}],"source_content_type":"text/x-python","patch_set":14,"id":"e192d5c9_7e8b6bc2","line":298,"in_reply_to":"8fb80023_4952b899","updated":"2022-07-15 17:50:03.000000000","message":"was meant to remove that. My bad","commit_id":"1b071aca6f1ff1a67962d7c0718957d05a391562"},{"author":{"_account_id":34208,"name":"Rafael Castillo","email":"rcastill@redhat.com","username":"rcastill"},"change_message_id":"5b7b22aca5de42a3e9273f281242adf3cc706db0","unresolved":false,"context_lines":[{"line_number":295,"context_line":"    def _needs_update(self, router, kwargs, external_fixed_ips, to_add, to_remove,"},{"line_number":296,"context_line":"                      missing_port_ids):"},{"line_number":297,"context_line":"        \"\"\"Decide if the given router needs an update.\"\"\""},{"line_number":298,"context_line":"        self.reason \u003d \u0027nope\u0027"},{"line_number":299,"context_line":""},{"line_number":300,"context_line":"        cur_ext_gw_info \u003d router[\u0027external_gateway_info\u0027]"},{"line_number":301,"context_line":"        if \u0027external_gateway_info\u0027 in kwargs:"}],"source_content_type":"text/x-python","patch_set":14,"id":"172966a4_3f2c8833","line":298,"in_reply_to":"e192d5c9_7e8b6bc2","updated":"2022-07-15 17:50:14.000000000","message":"Done","commit_id":"1b071aca6f1ff1a67962d7c0718957d05a391562"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"b710f3e3eb2b1c076b09d658081ee944ac27aae9","unresolved":true,"context_lines":[{"line_number":326,"context_line":"            ip \u003d fip.get(\u0027ip_address\u0027, None)"},{"line_number":327,"context_line":"            if subnet in cur_fip_map:"},{"line_number":328,"context_line":"                if ip is not None and ip not in cur_fip_map[subnet]:"},{"line_number":329,"context_line":"                    self.reason \u003d \"missmatching ip for subnet {0}, {1}\".format(subnet, cur_fip_map)"},{"line_number":330,"context_line":"                    return True"},{"line_number":331,"context_line":"            else:"},{"line_number":332,"context_line":"                self.reason \u003d \"adding ext ip with subnet {0}\".format(subnet)"}],"source_content_type":"text/x-python","patch_set":14,"id":"b9a5b265_f48696b7","line":329,"updated":"2022-07-15 08:37:25.000000000","message":"Returning the whole cur_fip_map by intention?","commit_id":"1b071aca6f1ff1a67962d7c0718957d05a391562"},{"author":{"_account_id":34208,"name":"Rafael Castillo","email":"rcastill@redhat.com","username":"rcastill"},"change_message_id":"661ca31501bdf8f75c664b9ba4554f987b69030e","unresolved":false,"context_lines":[{"line_number":326,"context_line":"            ip \u003d fip.get(\u0027ip_address\u0027, None)"},{"line_number":327,"context_line":"            if subnet in cur_fip_map:"},{"line_number":328,"context_line":"                if ip is not None and ip not in cur_fip_map[subnet]:"},{"line_number":329,"context_line":"                    self.reason \u003d \"missmatching ip for subnet {0}, {1}\".format(subnet, cur_fip_map)"},{"line_number":330,"context_line":"                    return True"},{"line_number":331,"context_line":"            else:"},{"line_number":332,"context_line":"                self.reason \u003d \"adding ext ip with subnet {0}\".format(subnet)"}],"source_content_type":"text/x-python","patch_set":14,"id":"696e1174_3fdb4961","line":329,"in_reply_to":"b9a5b265_f48696b7","updated":"2022-07-15 17:50:03.000000000","message":"Done","commit_id":"1b071aca6f1ff1a67962d7c0718957d05a391562"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"b710f3e3eb2b1c076b09d658081ee944ac27aae9","unresolved":true,"context_lines":[{"line_number":334,"context_line":"        # Check if external ip addresses need to be removed"},{"line_number":335,"context_line":"        for fip in cur_ext_fips:"},{"line_number":336,"context_line":"            subnet \u003d fip[\u0027subnet_id\u0027]"},{"line_number":337,"context_line":"            ip \u003d fip.get(\u0027ip_address\u0027, None)"},{"line_number":338,"context_line":"            if subnet in req_fip_map:"},{"line_number":339,"context_line":"                if ip not in req_fip_map[subnet]:"},{"line_number":340,"context_line":"                    self.reason \u003d \"removing ext ip with subnet (ip clash) {0}\".format(subnet)"}],"source_content_type":"text/x-python","patch_set":14,"id":"dc75e334_d576dfce","line":337,"updated":"2022-07-15 08:37:25.000000000","message":"Looking at OpenStack\u0027s API reference and your code below, we assert that \u0027ip_address\u0027 is always in fip. So maybe change this line to\n\n  ip \u003d fip[\u0027ip_address\u0027]\n  \nso that we definitely fail here if \u0027ip_address\u0027 is not in fip?","commit_id":"1b071aca6f1ff1a67962d7c0718957d05a391562"},{"author":{"_account_id":34208,"name":"Rafael Castillo","email":"rcastill@redhat.com","username":"rcastill"},"change_message_id":"661ca31501bdf8f75c664b9ba4554f987b69030e","unresolved":false,"context_lines":[{"line_number":334,"context_line":"        # Check if external ip addresses need to be removed"},{"line_number":335,"context_line":"        for fip in cur_ext_fips:"},{"line_number":336,"context_line":"            subnet \u003d fip[\u0027subnet_id\u0027]"},{"line_number":337,"context_line":"            ip \u003d fip.get(\u0027ip_address\u0027, None)"},{"line_number":338,"context_line":"            if subnet in req_fip_map:"},{"line_number":339,"context_line":"                if ip not in req_fip_map[subnet]:"},{"line_number":340,"context_line":"                    self.reason \u003d \"removing ext ip with subnet (ip clash) {0}\".format(subnet)"}],"source_content_type":"text/x-python","patch_set":14,"id":"2e14e2ed_261eaec1","line":337,"in_reply_to":"dc75e334_d576dfce","updated":"2022-07-15 17:50:03.000000000","message":"Done","commit_id":"1b071aca6f1ff1a67962d7c0718957d05a391562"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"b710f3e3eb2b1c076b09d658081ee944ac27aae9","unresolved":true,"context_lines":[{"line_number":345,"context_line":""},{"line_number":346,"context_line":"        if not external_fixed_ips and len(cur_ext_fips) \u003e 1:"},{"line_number":347,"context_line":"            # no external fixed ips requested but router has several external fixed ips"},{"line_number":348,"context_line":"            self.reason \u003d \"no external fixed ips requested but router has bla\""},{"line_number":349,"context_line":"            return True"},{"line_number":350,"context_line":""},{"line_number":351,"context_line":"        # check if internal interfaces need update"}],"source_content_type":"text/x-python","patch_set":14,"id":"ba3d3e48_7961a9fb","line":348,"updated":"2022-07-15 08:37:25.000000000","message":"\"has bla\"?","commit_id":"1b071aca6f1ff1a67962d7c0718957d05a391562"},{"author":{"_account_id":34208,"name":"Rafael Castillo","email":"rcastill@redhat.com","username":"rcastill"},"change_message_id":"661ca31501bdf8f75c664b9ba4554f987b69030e","unresolved":false,"context_lines":[{"line_number":345,"context_line":""},{"line_number":346,"context_line":"        if not external_fixed_ips and len(cur_ext_fips) \u003e 1:"},{"line_number":347,"context_line":"            # no external fixed ips requested but router has several external fixed ips"},{"line_number":348,"context_line":"            self.reason \u003d \"no external fixed ips requested but router has bla\""},{"line_number":349,"context_line":"            return True"},{"line_number":350,"context_line":""},{"line_number":351,"context_line":"        # check if internal interfaces need update"}],"source_content_type":"text/x-python","patch_set":14,"id":"07f6cc79_fa643b95","line":348,"in_reply_to":"ba3d3e48_7961a9fb","updated":"2022-07-15 17:50:03.000000000","message":"Done","commit_id":"1b071aca6f1ff1a67962d7c0718957d05a391562"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"b710f3e3eb2b1c076b09d658081ee944ac27aae9","unresolved":true,"context_lines":[{"line_number":409,"context_line":"                    self.fail(msg\u003d\u0027subnet %s not found\u0027 % iface[\u0027subnet\u0027])"},{"line_number":410,"context_line":"                fip \u003d dict(subnet_id\u003dsubnet.id)"},{"line_number":411,"context_line":"                if \u0027ip_address\u0027 in iface:"},{"line_number":412,"context_line":"                    fip[\u0027ip_address\u0027] \u003d iface[\u0027ip_address\u0027]"},{"line_number":413,"context_line":"                external_fixed_ips.append(fip)"},{"line_number":414,"context_line":""},{"line_number":415,"context_line":"        # Build internal interface configuration"}],"source_content_type":"text/x-python","patch_set":14,"id":"8b6250ef_d9aab7e1","line":412,"updated":"2022-07-15 08:37:25.000000000","message":"Please add for backward compatibility:\n\n                elif \u0027ip\u0027 in iface:\n                    # for backward compatibility\n                    fip[\u0027ip_address\u0027] \u003d iface[\u0027ip\u0027]","commit_id":"1b071aca6f1ff1a67962d7c0718957d05a391562"},{"author":{"_account_id":34208,"name":"Rafael Castillo","email":"rcastill@redhat.com","username":"rcastill"},"change_message_id":"661ca31501bdf8f75c664b9ba4554f987b69030e","unresolved":true,"context_lines":[{"line_number":409,"context_line":"                    self.fail(msg\u003d\u0027subnet %s not found\u0027 % iface[\u0027subnet\u0027])"},{"line_number":410,"context_line":"                fip \u003d dict(subnet_id\u003dsubnet.id)"},{"line_number":411,"context_line":"                if \u0027ip_address\u0027 in iface:"},{"line_number":412,"context_line":"                    fip[\u0027ip_address\u0027] \u003d iface[\u0027ip_address\u0027]"},{"line_number":413,"context_line":"                external_fixed_ips.append(fip)"},{"line_number":414,"context_line":""},{"line_number":415,"context_line":"        # Build internal interface configuration"}],"source_content_type":"text/x-python","patch_set":14,"id":"e083c392_3dbb9a57","line":412,"in_reply_to":"8b6250ef_d9aab7e1","updated":"2022-07-15 17:50:03.000000000","message":"They\u0027re already aliased in the argspec, no? do aliases not work when nested this deep?","commit_id":"1b071aca6f1ff1a67962d7c0718957d05a391562"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"baebc126f0baa140b9bcac418383fa9ca2a61466","unresolved":false,"context_lines":[{"line_number":409,"context_line":"                    self.fail(msg\u003d\u0027subnet %s not found\u0027 % iface[\u0027subnet\u0027])"},{"line_number":410,"context_line":"                fip \u003d dict(subnet_id\u003dsubnet.id)"},{"line_number":411,"context_line":"                if \u0027ip_address\u0027 in iface:"},{"line_number":412,"context_line":"                    fip[\u0027ip_address\u0027] \u003d iface[\u0027ip_address\u0027]"},{"line_number":413,"context_line":"                external_fixed_ips.append(fip)"},{"line_number":414,"context_line":""},{"line_number":415,"context_line":"        # Build internal interface configuration"}],"source_content_type":"text/x-python","patch_set":14,"id":"e52041d0_e13399c3","line":412,"in_reply_to":"e083c392_3dbb9a57","updated":"2022-07-18 11:51:44.000000000","message":"Ah! Aliases for suboptions are supported in the devel [1] branch of Ansible down to at least Ansible 2.9 [2]. Nice :)\n\n[1] https://github.com/ansible/ansible/blob/b63812bc08fd00fd772c28a2604f77f487d23104/lib/ansible/module_utils/common/parameters.py#L745\n[2] https://github.com/ansible/ansible/blob/06886f09c5c05a93337637028ba9e7670c8e0493/lib/ansible/module_utils/basic.py#L1827","commit_id":"1b071aca6f1ff1a67962d7c0718957d05a391562"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"973120bbbc9698367d58ec5a337adbb2d8753ce6","unresolved":false,"context_lines":[{"line_number":449,"context_line":"                        for port in existing_ports:"},{"line_number":450,"context_line":"                            for fixed_ip in port[\u0027fixed_ips\u0027]:"},{"line_number":451,"context_line":"                                if (portip \u003d\u003d fixed_ip[\u0027ip_address\u0027]"},{"line_number":452,"context_line":"                                        and fixed_ip[\u0027subnet_id\u0027] \u003d\u003d subnet.id):"},{"line_number":453,"context_line":"                                    # portip exists in net already"},{"line_number":454,"context_line":"                                    internal_ips.append(fixed_ip[\u0027ip_address\u0027])"},{"line_number":455,"context_line":"                                    internal_ifaces.append("}],"source_content_type":"text/x-python","patch_set":14,"id":"4b5e4f78_c1e3f1ba","line":452,"updated":"2022-07-14 12:02:11.000000000","message":"Good catch 😊","commit_id":"1b071aca6f1ff1a67962d7c0718957d05a391562"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"e018ffc091c0ba2cef32b174a07848b53aa7a955","unresolved":true,"context_lines":[{"line_number":530,"context_line":"            obsolete_subnet_ids \u003d set(existing_subnet_ids) - set(requested_subnet_ids)"},{"line_number":531,"context_line":"            existing_port_ids \u003d [i[\u0027id\u0027] for i in router_ifs_internal]"},{"line_number":532,"context_line":""},{"line_number":533,"context_line":"        missing_port_ids \u003d set(requested_port_ids) - set(existing_port_ids)"},{"line_number":534,"context_line":""},{"line_number":535,"context_line":"        if self.ansible.check_mode:"},{"line_number":536,"context_line":"            # Check if the system state would be changed"}],"source_content_type":"text/x-python","patch_set":14,"id":"40f68956_bcc2e1d3","line":533,"updated":"2022-07-18 12:33:15.000000000","message":"Suppose two ports with ip 10.1.1.10 and 10.1.1.11 exist in subnet private-subnet, but they have not been attached to any device. Now, a user runs:\n\n  - openstack.cloud.router:\n    cloud: mycloud\n    state: present\n    name: router2\n    network: ext_network1\n    interfaces:\n      - net: private-net\n        subnet: private-subnet\n        portip: 10.1.1.10\n\nLater, the user runs:\n\n  - openstack.cloud.router:\n    cloud: mycloud\n    state: present\n    name: router2\n    network: ext_network1\n    interfaces:\n      - net: private-net\n        subnet: private-subnet\n        portip: 10.1.1.10\n      - net: private-net\n        subnet: private-subnet\n        portip: 10.1.1.11\n\nWill the second port be added to the router since missing_port_ids has been dropped?","commit_id":"1b071aca6f1ff1a67962d7c0718957d05a391562"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"48b8db00623b1a08299810b68e11e0ddba6a7b58","unresolved":false,"context_lines":[{"line_number":530,"context_line":"            obsolete_subnet_ids \u003d set(existing_subnet_ids) - set(requested_subnet_ids)"},{"line_number":531,"context_line":"            existing_port_ids \u003d [i[\u0027id\u0027] for i in router_ifs_internal]"},{"line_number":532,"context_line":""},{"line_number":533,"context_line":"        missing_port_ids \u003d set(requested_port_ids) - set(existing_port_ids)"},{"line_number":534,"context_line":""},{"line_number":535,"context_line":"        if self.ansible.check_mode:"},{"line_number":536,"context_line":"            # Check if the system state would be changed"}],"source_content_type":"text/x-python","patch_set":14,"id":"70c741d2_e503ff73","line":533,"in_reply_to":"2beeacd0_78ecce5c","updated":"2022-07-28 09:30:04.000000000","message":"Thank you very much!","commit_id":"1b071aca6f1ff1a67962d7c0718957d05a391562"},{"author":{"_account_id":34208,"name":"Rafael Castillo","email":"rcastill@redhat.com","username":"rcastill"},"change_message_id":"e85534409f70f7ed1c3e6fc962b364dcb3172935","unresolved":true,"context_lines":[{"line_number":530,"context_line":"            obsolete_subnet_ids \u003d set(existing_subnet_ids) - set(requested_subnet_ids)"},{"line_number":531,"context_line":"            existing_port_ids \u003d [i[\u0027id\u0027] for i in router_ifs_internal]"},{"line_number":532,"context_line":""},{"line_number":533,"context_line":"        missing_port_ids \u003d set(requested_port_ids) - set(existing_port_ids)"},{"line_number":534,"context_line":""},{"line_number":535,"context_line":"        if self.ansible.check_mode:"},{"line_number":536,"context_line":"            # Check if the system state would be changed"}],"source_content_type":"text/x-python","patch_set":14,"id":"aa0f7791_60b04fdd","line":533,"in_reply_to":"40f68956_bcc2e1d3","updated":"2022-07-18 15:41:27.000000000","message":"Not sure how valid this specification is. In my tests, the neutron API returns a \"400: Bad router request: Router already has a port on subnet\" error when I try to do it this way.\n\nNot sure what to do in this case. Let it throw an error (or warning?)? merge the two addresses into a single port? Right now it\u0027ll silently ignore one.","commit_id":"1b071aca6f1ff1a67962d7c0718957d05a391562"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"8d01548fb32ac9053964dc543925319969b0def0","unresolved":true,"context_lines":[{"line_number":530,"context_line":"            obsolete_subnet_ids \u003d set(existing_subnet_ids) - set(requested_subnet_ids)"},{"line_number":531,"context_line":"            existing_port_ids \u003d [i[\u0027id\u0027] for i in router_ifs_internal]"},{"line_number":532,"context_line":""},{"line_number":533,"context_line":"        missing_port_ids \u003d set(requested_port_ids) - set(existing_port_ids)"},{"line_number":534,"context_line":""},{"line_number":535,"context_line":"        if self.ansible.check_mode:"},{"line_number":536,"context_line":"            # Check if the system state would be changed"}],"source_content_type":"text/x-python","patch_set":14,"id":"b5dd5fa1_8e6ea3eb","line":533,"in_reply_to":"aa0f7791_60b04fdd","updated":"2022-07-19 08:36:03.000000000","message":"Thanks for testing this! Actually Neutron never allowed a router to be attached twice to the same subnet [1][2].\n\n[1] https://opendev.org/openstack/neutron/commit/9355885d7dab41e48a8ecaaf59b1bd3835f17f27\n[2] https://opendev.org/openstack/neutron/commit/3005d16fe3588bdf4b928e79f640b991df9fae3b\n\nBut this portip attribute introduces a lot more cases which we do not currently do not implement. For example, suppose the user runs:\n\n  - openstack.cloud.router:\n    cloud: mycloud\n    state: present\n    name: router2\n    network: ext_network1\n    interfaces:\n      - net: private-net\n        subnet: private-subnet\n\nand this will auto-assign portip 10.1.1.10 on private-subnet to router2. Later, the user runs:\n\n  - openstack.cloud.router:\n    cloud: mycloud\n    state: present\n    name: router2\n    network: ext_network1\n    interfaces:\n      - net: private-net\n        subnet: private-subnet\n        portip: 10.1.1.11\n\nOne would expect our module to detach/remove the port with ip 10.1.1.10 and assign 10.1.1.11 to router2. So we should (re)add some logic to add or remove specific ports/portips. Once we have that, we let Neutron handle the case of two ports on the same subnet, i.e. we let Neutron report an error on that.","commit_id":"1b071aca6f1ff1a67962d7c0718957d05a391562"},{"author":{"_account_id":34208,"name":"Rafael Castillo","email":"rcastill@redhat.com","username":"rcastill"},"change_message_id":"c5843ef416488f216b060f98a23cfdcf6f474b4a","unresolved":true,"context_lines":[{"line_number":530,"context_line":"            obsolete_subnet_ids \u003d set(existing_subnet_ids) - set(requested_subnet_ids)"},{"line_number":531,"context_line":"            existing_port_ids \u003d [i[\u0027id\u0027] for i in router_ifs_internal]"},{"line_number":532,"context_line":""},{"line_number":533,"context_line":"        missing_port_ids \u003d set(requested_port_ids) - set(existing_port_ids)"},{"line_number":534,"context_line":""},{"line_number":535,"context_line":"        if self.ansible.check_mode:"},{"line_number":536,"context_line":"            # Check if the system state would be changed"}],"source_content_type":"text/x-python","patch_set":14,"id":"2beeacd0_78ecce5c","line":533,"in_reply_to":"b5dd5fa1_8e6ea3eb","updated":"2022-07-22 05:29:33.000000000","message":"I\u0027ve gone ahead and added logic to handle that case, as well as a test case that exercises it.","commit_id":"1b071aca6f1ff1a67962d7c0718957d05a391562"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"b710f3e3eb2b1c076b09d658081ee944ac27aae9","unresolved":true,"context_lines":[{"line_number":541,"context_line":"            elif state \u003d\u003d \u0027present\u0027 and not router:"},{"line_number":542,"context_line":"                changed \u003d True"},{"line_number":543,"context_line":"            else:  # if state \u003d\u003d \u0027present\u0027 and router"},{"line_number":544,"context_line":"                changed \u003d self._needs_update(router, network,"},{"line_number":545,"context_line":"                                             missing_port_ids,"},{"line_number":546,"context_line":"                                             requested_subnet_ids,"},{"line_number":547,"context_line":"                                             existing_subnet_ids,"}],"source_content_type":"text/x-python","patch_set":14,"id":"a65d0e27_a46c46dc","line":544,"updated":"2022-07-15 08:37:25.000000000","message":"Please update this call to self._needs_update with your updated function parameters. For example, the third parameter \u0027missing_port_ids\u0027 has been replaced with \u0027external_fixed_ips\u0027, see lines 295 and 582.","commit_id":"1b071aca6f1ff1a67962d7c0718957d05a391562"},{"author":{"_account_id":34208,"name":"Rafael Castillo","email":"rcastill@redhat.com","username":"rcastill"},"change_message_id":"661ca31501bdf8f75c664b9ba4554f987b69030e","unresolved":false,"context_lines":[{"line_number":541,"context_line":"            elif state \u003d\u003d \u0027present\u0027 and not router:"},{"line_number":542,"context_line":"                changed \u003d True"},{"line_number":543,"context_line":"            else:  # if state \u003d\u003d \u0027present\u0027 and router"},{"line_number":544,"context_line":"                changed \u003d self._needs_update(router, network,"},{"line_number":545,"context_line":"                                             missing_port_ids,"},{"line_number":546,"context_line":"                                             requested_subnet_ids,"},{"line_number":547,"context_line":"                                             existing_subnet_ids,"}],"source_content_type":"text/x-python","patch_set":14,"id":"1252848a_ba506221","line":544,"in_reply_to":"a65d0e27_a46c46dc","updated":"2022-07-15 17:50:03.000000000","message":"Done","commit_id":"1b071aca6f1ff1a67962d7c0718957d05a391562"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"83d19a29bc792c7f54cd40688af8d0231a5098f5","unresolved":true,"context_lines":[{"line_number":479,"context_line":"            network_name_or_id \u003d self.params[\u0027external_gateway_info\u0027][\u0027network\u0027]"},{"line_number":480,"context_line":"        return network_name_or_id"},{"line_number":481,"context_line":""},{"line_number":482,"context_line":"    def get_port_changes(self, router, ifs_cfg):"},{"line_number":483,"context_line":"        requested_subnet_ids \u003d [iface[\u0027subnet_id\u0027] for iface"},{"line_number":484,"context_line":"                                in ifs_cfg[\u0027internal_ifaces\u0027]]"},{"line_number":485,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"1a7b020e_47b91f5b","line":482,"updated":"2022-07-18 12:34:13.000000000","message":"nit: get_port_changes \u003d\u003e _get_port_changes?","commit_id":"31011da822d8f3f8884943469e25f9434bc49ddf"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"dc163adb99c1508f34c902e0f7ef42d394d6e302","unresolved":false,"context_lines":[{"line_number":479,"context_line":"            network_name_or_id \u003d self.params[\u0027external_gateway_info\u0027][\u0027network\u0027]"},{"line_number":480,"context_line":"        return network_name_or_id"},{"line_number":481,"context_line":""},{"line_number":482,"context_line":"    def get_port_changes(self, router, ifs_cfg):"},{"line_number":483,"context_line":"        requested_subnet_ids \u003d [iface[\u0027subnet_id\u0027] for iface"},{"line_number":484,"context_line":"                                in ifs_cfg[\u0027internal_ifaces\u0027]]"},{"line_number":485,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"80f74a6e_e9be8480","line":482,"in_reply_to":"1a7b020e_47b91f5b","updated":"2022-07-25 09:33:37.000000000","message":"Done, thanks!","commit_id":"31011da822d8f3f8884943469e25f9434bc49ddf"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"79b41e745f8080f6b581a9e5174907e080f873f2","unresolved":true,"context_lines":[{"line_number":409,"context_line":"                    subnet \u003d self.conn.network.find_subnet(iface, **filters)"},{"line_number":410,"context_line":"                    if not subnet:"},{"line_number":411,"context_line":"                        self.fail(msg\u003d\u0027subnet %s not found\u0027 % iface)"},{"line_number":412,"context_line":"                    internal_ifaces.append(dict(subnet_id\u003dsubnet.id))"},{"line_number":413,"context_line":""},{"line_number":414,"context_line":"                elif isinstance(iface, dict):"},{"line_number":415,"context_line":"                    subnet \u003d self.conn.network.find_subnet(iface[\u0027subnet\u0027],"}],"source_content_type":"text/x-python","patch_set":20,"id":"f284629c_1c0fc089","line":412,"updated":"2022-07-25 09:37:27.000000000","message":"This line of code adds the subnet shade_subnet1 from comment [1] to the list of interfaces. But method _get_port_changes() has no code to actually add this subnet.\n\n[1] https://review.opendev.org/c/openstack/ansible-collections-openstack/+/846446/comments/9d1c708c_99d9d507","commit_id":"02fb83674ac625c53947296a590d31da0591787a"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"a349a9e51a643da44dc42a0bea2c623001dac056","unresolved":false,"context_lines":[{"line_number":409,"context_line":"                    subnet \u003d self.conn.network.find_subnet(iface, **filters)"},{"line_number":410,"context_line":"                    if not subnet:"},{"line_number":411,"context_line":"                        self.fail(msg\u003d\u0027subnet %s not found\u0027 % iface)"},{"line_number":412,"context_line":"                    internal_ifaces.append(dict(subnet_id\u003dsubnet.id))"},{"line_number":413,"context_line":""},{"line_number":414,"context_line":"                elif isinstance(iface, dict):"},{"line_number":415,"context_line":"                    subnet \u003d self.conn.network.find_subnet(iface[\u0027subnet\u0027],"}],"source_content_type":"text/x-python","patch_set":20,"id":"db593df1_7eca8944","line":412,"in_reply_to":"f284629c_1c0fc089","updated":"2022-07-25 09:49:47.000000000","message":"_get_port_changes() has code to add this subnet [1].\n\n[1] https://review.opendev.org/c/openstack/ansible-collections-openstack/+/846446/20/plugins/modules/router.py#514","commit_id":"02fb83674ac625c53947296a590d31da0591787a"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"1cdb3753c2d36a794b86fdaa0efc7aa837a34a83","unresolved":true,"context_lines":[{"line_number":417,"context_line":"                    if not subnet:"},{"line_number":418,"context_line":"                        self.fail(msg\u003d\u0027subnet %s not found\u0027 % iface[\u0027subnet\u0027])"},{"line_number":419,"context_line":""},{"line_number":420,"context_line":"                    if \u0027net\u0027 not in iface:"},{"line_number":421,"context_line":"                        self.fail("},{"line_number":422,"context_line":"                            \"Network name missing from interface definition\")"},{"line_number":423,"context_line":"                    net \u003d self.conn.network.find_network(iface[\u0027net\u0027])"}],"source_content_type":"text/x-python","patch_set":20,"id":"024bba19_62ce4b31","line":420,"updated":"2022-07-25 10:00:26.000000000","message":"We allow to pass a subnet without giving a network above in \"if isinstance(iface, str):\" [1] hence we should allow to omit the network here as well.\n\n(We can do this refactoring later, let us just keep this comment non-resolved as a TODO reminder)\n\n[1] https://review.opendev.org/c/openstack/ansible-collections-openstack/+/846446/20/plugins/modules/router.py#408","commit_id":"02fb83674ac625c53947296a590d31da0591787a"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"590325e35db4a85252d0708ffa0ef5b218b9727b","unresolved":false,"context_lines":[{"line_number":417,"context_line":"                    if not subnet:"},{"line_number":418,"context_line":"                        self.fail(msg\u003d\u0027subnet %s not found\u0027 % iface[\u0027subnet\u0027])"},{"line_number":419,"context_line":""},{"line_number":420,"context_line":"                    if \u0027net\u0027 not in iface:"},{"line_number":421,"context_line":"                        self.fail("},{"line_number":422,"context_line":"                            \"Network name missing from interface definition\")"},{"line_number":423,"context_line":"                    net \u003d self.conn.network.find_network(iface[\u0027net\u0027])"}],"source_content_type":"text/x-python","patch_set":20,"id":"13b4511f_ab8dfc2f","line":420,"in_reply_to":"024bba19_62ce4b31","updated":"2022-07-28 08:26:39.000000000","message":"Added it as an inline todo because it is unlikely we look at these comments here again once this patch has been merged.","commit_id":"02fb83674ac625c53947296a590d31da0591787a"}]}