)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":27900,"name":"Artem Goncharov","email":"artem.goncharov@gmail.com","username":"gtema"},"change_message_id":"3d44e1eb02e85e585fc5dcc19c12f8b475614a36","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"f47ec764_ddcbb447","updated":"2023-01-11 16:35:31.000000000","message":"this is something I was also thinking about for quite some time already. For that usecase (actually to heavily streamline _info modules) I have added those _resource_registry in sdk. I like the idea","commit_id":"e05191e74228b084f24e01d8e5785916dd3c6915"},{"author":{"_account_id":27900,"name":"Artem Goncharov","email":"artem.goncharov@gmail.com","username":"gtema"},"change_message_id":"edf007e36bde421aca392ffc6658adaae93bef0e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"e76feacc_7593cabd","updated":"2023-01-25 18:49:28.000000000","message":"I like it. There are few places that require certain improvements, but can be done also later.","commit_id":"ef4652f226be351a2555e7fc2a235c6a35e917a5"}],"ci/roles/resource/tasks/main.yml":[{"author":{"_account_id":27900,"name":"Artem Goncharov","email":"artem.goncharov@gmail.com","username":"gtema"},"change_message_id":"edf007e36bde421aca392ffc6658adaae93bef0e","unresolved":true,"context_lines":[{"line_number":285,"context_line":"        that:"},{"line_number":286,"context_line":"          - router is not changed"},{"line_number":287,"context_line":""},{"line_number":288,"context_line":"    - name: Attach router to internal subnet"},{"line_number":289,"context_line":"      openstack.cloud.router:"},{"line_number":290,"context_line":"        name: ansible_router"},{"line_number":291,"context_line":"        network: \"{{ network_external.resource.id }}\""}],"source_content_type":"text/x-yaml","patch_set":16,"id":"78489b16_4f2c249c","line":288,"updated":"2023-01-25 18:49:28.000000000","message":"generally using other modules in tests is ok, but here we are testing a very generic thing. If we will try to rework router module to rely on the resource/SM as well we may end up in a endless loop.\nI would prefer if we do not rely on any other modules in tests of groundbreaking things.","commit_id":"ef4652f226be351a2555e7fc2a235c6a35e917a5"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"c2a24d809c9a49dd8c3e0603bef1f5f65387cfce","unresolved":false,"context_lines":[{"line_number":285,"context_line":"        that:"},{"line_number":286,"context_line":"          - router is not changed"},{"line_number":287,"context_line":""},{"line_number":288,"context_line":"    - name: Attach router to internal subnet"},{"line_number":289,"context_line":"      openstack.cloud.router:"},{"line_number":290,"context_line":"        name: ansible_router"},{"line_number":291,"context_line":"        network: \"{{ network_external.resource.id }}\""}],"source_content_type":"text/x-yaml","patch_set":16,"id":"0ff16576_bd0f4244","line":288,"in_reply_to":"78489b16_4f2c249c","updated":"2023-01-25 19:44:16.000000000","message":"I agree with your dislike for using the router module here. Unfortunately we have to rely on router module here because openstacksdk does not offer a crud-ified resource class which would allow us to attach and detach router interfaces. It would be a very welcome addition to openstacksdk though!\n\nActually we have several places in ci where we use other modules than the ones we want to test within a ci role. For example one cannot create a floating ip address without having networks first. But this is totally fine. This is CI only, so we cannot really produce loops which break us.\n\nSo once openstacksdk gains a crud api for router interfaces this router call should definitely be replaced.","commit_id":"ef4652f226be351a2555e7fc2a235c6a35e917a5"}],"plugins/module_utils/resource.py":[{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"000a389e01c1a8533365f5b74fa0a69788bc7413","unresolved":true,"context_lines":[{"line_number":217,"context_line":""},{"line_number":218,"context_line":"            return resource, False  # would be bool(update) without check_mode"},{"line_number":219,"context_line":"        elif state \u003d\u003d \u0027absent\u0027 and resource:"},{"line_number":220,"context_line":"            return None, False  # would be True without check_mode"},{"line_number":221,"context_line":"        else:"},{"line_number":222,"context_line":"            # state \u003d\u003d \u0027absent\u0027 and not resource:"},{"line_number":223,"context_line":"            return None, False"}],"source_content_type":"text/x-python","patch_set":12,"id":"901dff42_3c5d13d5","line":220,"updated":"2023-01-16 13:39:48.000000000","message":"Should we return \"changed\u003dTrue\" in check_mode? Or shall we always return \"changed\u003dFalse\"?","commit_id":"aff7c94521e848727c594553bc1c0b4efb6bcd9f"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"8bf15b37f5a2e91ee3594061c1c54bd0c31d31a9","unresolved":false,"context_lines":[{"line_number":217,"context_line":""},{"line_number":218,"context_line":"            return resource, False  # would be bool(update) without check_mode"},{"line_number":219,"context_line":"        elif state \u003d\u003d \u0027absent\u0027 and resource:"},{"line_number":220,"context_line":"            return None, False  # would be True without check_mode"},{"line_number":221,"context_line":"        else:"},{"line_number":222,"context_line":"            # state \u003d\u003d \u0027absent\u0027 and not resource:"},{"line_number":223,"context_line":"            return None, False"}],"source_content_type":"text/x-python","patch_set":12,"id":"c7e97bd4_e3ae2ad4","line":220,"in_reply_to":"901dff42_3c5d13d5","updated":"2023-01-16 18:46:38.000000000","message":"Ansible\u0027s own modules such as apt, file, lineinfile, systemd_service and group all return a proper changed value when running in check_mode.","commit_id":"aff7c94521e848727c594553bc1c0b4efb6bcd9f"},{"author":{"_account_id":27900,"name":"Artem Goncharov","email":"artem.goncharov@gmail.com","username":"gtema"},"change_message_id":"edf007e36bde421aca392ffc6658adaae93bef0e","unresolved":true,"context_lines":[{"line_number":117,"context_line":"        if wait:"},{"line_number":118,"context_line":"            resource \u003d self.sdk.resource.wait_for_status(self.session,"},{"line_number":119,"context_line":"                                                         resource,"},{"line_number":120,"context_line":"                                                         status\u003d\u0027active\u0027,"},{"line_number":121,"context_line":"                                                         failures\u003d[\u0027error\u0027],"},{"line_number":122,"context_line":"                                                         wait\u003dtimeout,"},{"line_number":123,"context_line":"                                                         attribute\u003d\u0027status\u0027)"}],"source_content_type":"text/x-python","patch_set":16,"id":"1a48ca75_4ef215a4","line":120,"updated":"2023-01-25 18:49:28.000000000","message":"I guess we should add possibility here and everywhere else to override desired status/failures/status. In most of cases we have them standardized, but iirc there were some funny exceptions (servers and volume attachments may be tricky).","commit_id":"ef4652f226be351a2555e7fc2a235c6a35e917a5"},{"author":{"_account_id":27900,"name":"Artem Goncharov","email":"artem.goncharov@gmail.com","username":"gtema"},"change_message_id":"d25120428f16d89e876ff7ce720242815085dcfc","unresolved":false,"context_lines":[{"line_number":117,"context_line":"        if wait:"},{"line_number":118,"context_line":"            resource \u003d self.sdk.resource.wait_for_status(self.session,"},{"line_number":119,"context_line":"                                                         resource,"},{"line_number":120,"context_line":"                                                         status\u003d\u0027active\u0027,"},{"line_number":121,"context_line":"                                                         failures\u003d[\u0027error\u0027],"},{"line_number":122,"context_line":"                                                         wait\u003dtimeout,"},{"line_number":123,"context_line":"                                                         attribute\u003d\u0027status\u0027)"}],"source_content_type":"text/x-python","patch_set":16,"id":"0159f6c1_ca22659b","line":120,"in_reply_to":"16a5982f_497cf373","updated":"2023-01-26 12:07:19.000000000","message":"Done","commit_id":"ef4652f226be351a2555e7fc2a235c6a35e917a5"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"c2a24d809c9a49dd8c3e0603bef1f5f65387cfce","unresolved":true,"context_lines":[{"line_number":117,"context_line":"        if wait:"},{"line_number":118,"context_line":"            resource \u003d self.sdk.resource.wait_for_status(self.session,"},{"line_number":119,"context_line":"                                                         resource,"},{"line_number":120,"context_line":"                                                         status\u003d\u0027active\u0027,"},{"line_number":121,"context_line":"                                                         failures\u003d[\u0027error\u0027],"},{"line_number":122,"context_line":"                                                         wait\u003dtimeout,"},{"line_number":123,"context_line":"                                                         attribute\u003d\u0027status\u0027)"}],"source_content_type":"text/x-python","patch_set":16,"id":"4c031126_51913894","line":120,"in_reply_to":"1a48ca75_4ef215a4","updated":"2023-01-25 19:44:16.000000000","message":"Agree. Let me add a TODO for this improvement as future work.","commit_id":"ef4652f226be351a2555e7fc2a235c6a35e917a5"},{"author":{"_account_id":27900,"name":"Artem Goncharov","email":"artem.goncharov@gmail.com","username":"gtema"},"change_message_id":"a06eecbf35589e57c72048532c3d7b02416f0b54","unresolved":true,"context_lines":[{"line_number":117,"context_line":"        if wait:"},{"line_number":118,"context_line":"            resource \u003d self.sdk.resource.wait_for_status(self.session,"},{"line_number":119,"context_line":"                                                         resource,"},{"line_number":120,"context_line":"                                                         status\u003d\u0027active\u0027,"},{"line_number":121,"context_line":"                                                         failures\u003d[\u0027error\u0027],"},{"line_number":122,"context_line":"                                                         wait\u003dtimeout,"},{"line_number":123,"context_line":"                                                         attribute\u003d\u0027status\u0027)"}],"source_content_type":"text/x-python","patch_set":16,"id":"16a5982f_497cf373","line":120,"in_reply_to":"3a89937a_645263c4","updated":"2023-01-26 08:29:34.000000000","message":"In principle yes. Actually at the very beginning I was not expecting you would switch all modules to the SM internals therefore was more or less evaluating the module in the scope of a standalone module. But I agree, this is better to be overridden by individual modules.","commit_id":"ef4652f226be351a2555e7fc2a235c6a35e917a5"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"0891d283db6ead4a9dad1e92a21ea83e3c356b17","unresolved":true,"context_lines":[{"line_number":117,"context_line":"        if wait:"},{"line_number":118,"context_line":"            resource \u003d self.sdk.resource.wait_for_status(self.session,"},{"line_number":119,"context_line":"                                                         resource,"},{"line_number":120,"context_line":"                                                         status\u003d\u0027active\u0027,"},{"line_number":121,"context_line":"                                                         failures\u003d[\u0027error\u0027],"},{"line_number":122,"context_line":"                                                         wait\u003dtimeout,"},{"line_number":123,"context_line":"                                                         attribute\u003d\u0027status\u0027)"}],"source_content_type":"text/x-python","patch_set":16,"id":"3a89937a_645263c4","line":120,"in_reply_to":"4c031126_51913894","updated":"2023-01-26 08:27:04.000000000","message":"Perhaps a better solution is to create a subclass of StateMachine and override specific functions such as _create(), _update() or _delete() as shown in [1],[2],[3],[4]. This approach saves us from code bloat in the resource module. \n\nAdditional module parameters for customizing \u0027status\u0027 and \u0027attribute\u0027 in wait_for_status() would require a knowing openstacksdk internals and thus only useful for a small circle of people who could just fix existing modules instead ;)\n\n[1] https://review.opendev.org/c/openstack/ansible-collections-openstack/+/870047/\n[2] https://review.opendev.org/c/openstack/ansible-collections-openstack/+/870070/\n[3] https://review.opendev.org/c/openstack/ansible-collections-openstack/+/870072/\n[4] https://review.opendev.org/c/openstack/ansible-collections-openstack/+/870087/","commit_id":"ef4652f226be351a2555e7fc2a235c6a35e917a5"}],"plugins/modules/resource.py":[{"author":{"_account_id":27900,"name":"Artem Goncharov","email":"artem.goncharov@gmail.com","username":"gtema"},"change_message_id":"3d44e1eb02e85e585fc5dcc19c12f8b475614a36","unresolved":true,"context_lines":[{"line_number":376,"context_line":"class ResourceModule(OpenStackModule):"},{"line_number":377,"context_line":"    argument_spec \u003d dict("},{"line_number":378,"context_line":"        attributes\u003ddict(required\u003dTrue, type\u003d\u0027dict\u0027),"},{"line_number":379,"context_line":"        component\u003ddict(required\u003dTrue),"},{"line_number":380,"context_line":"        non_updateable_attributes\u003ddict(type\u003d\u0027list\u0027, elements\u003d\u0027str\u0027),"},{"line_number":381,"context_line":"        state\u003ddict(default\u003d\u0027present\u0027, choices\u003d[\u0027absent\u0027, \u0027present\u0027]),"},{"line_number":382,"context_line":"        type\u003ddict(required\u003dTrue),"}],"source_content_type":"text/x-python","patch_set":3,"id":"8cd07f3b_d9ea6fe4","line":379,"updated":"2023-01-11 16:35:31.000000000","message":"I would rename this to service. This is going very much to the direction of https://opendev.org/openstack/openstacksdk/src/branch/master/openstack/block_storage/v3/_proxy.py#L34 where each service proxy registers supported resources","commit_id":"e05191e74228b084f24e01d8e5785916dd3c6915"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"063cb9090016f84f4cf96c0eda80507a1b10dbb5","unresolved":false,"context_lines":[{"line_number":376,"context_line":"class ResourceModule(OpenStackModule):"},{"line_number":377,"context_line":"    argument_spec \u003d dict("},{"line_number":378,"context_line":"        attributes\u003ddict(required\u003dTrue, type\u003d\u0027dict\u0027),"},{"line_number":379,"context_line":"        component\u003ddict(required\u003dTrue),"},{"line_number":380,"context_line":"        non_updateable_attributes\u003ddict(type\u003d\u0027list\u0027, elements\u003d\u0027str\u0027),"},{"line_number":381,"context_line":"        state\u003ddict(default\u003d\u0027present\u0027, choices\u003d[\u0027absent\u0027, \u0027present\u0027]),"},{"line_number":382,"context_line":"        type\u003ddict(required\u003dTrue),"}],"source_content_type":"text/x-python","patch_set":3,"id":"d15e92da_8d7c57d9","line":379,"in_reply_to":"8cd07f3b_d9ea6fe4","updated":"2023-01-11 19:52:38.000000000","message":"Thanks, service is a better name! Done.","commit_id":"e05191e74228b084f24e01d8e5785916dd3c6915"},{"author":{"_account_id":27900,"name":"Artem Goncharov","email":"artem.goncharov@gmail.com","username":"gtema"},"change_message_id":"edf007e36bde421aca392ffc6658adaae93bef0e","unresolved":true,"context_lines":[{"line_number":65,"context_line":"         available resource classes such as C(Server) inside C(*.py) files.\""},{"line_number":66,"context_line":"    required: true"},{"line_number":67,"context_line":"    type: str"},{"line_number":68,"context_line":"  updateable_attributes:"},{"line_number":69,"context_line":"    description:"},{"line_number":70,"context_line":"      - List of attribute names which can be updated."},{"line_number":71,"context_line":"      - When I(updateable_attributes) is not specified, then all attributes"}],"source_content_type":"text/x-python","patch_set":16,"id":"671403ea_a629a719","line":68,"updated":"2023-01-25 18:49:28.000000000","message":"I think it is going to very very confusing for users to understand purpose of this. docstring is not clear enough. updateable_attributes - non_updateable_attributes is not mentioned","commit_id":"ef4652f226be351a2555e7fc2a235c6a35e917a5"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"c2a24d809c9a49dd8c3e0603bef1f5f65387cfce","unresolved":true,"context_lines":[{"line_number":65,"context_line":"         available resource classes such as C(Server) inside C(*.py) files.\""},{"line_number":66,"context_line":"    required: true"},{"line_number":67,"context_line":"    type: str"},{"line_number":68,"context_line":"  updateable_attributes:"},{"line_number":69,"context_line":"    description:"},{"line_number":70,"context_line":"      - List of attribute names which can be updated."},{"line_number":71,"context_line":"      - When I(updateable_attributes) is not specified, then all attributes"}],"source_content_type":"text/x-python","patch_set":16,"id":"cb438a48_5d8e9d07","line":68,"in_reply_to":"671403ea_a629a719","updated":"2023-01-25 19:44:16.000000000","message":"Any suggestions for strings we could add to description here to make it more clear?","commit_id":"ef4652f226be351a2555e7fc2a235c6a35e917a5"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"bb706fca204eb1ea193a11c5387db60e81e044db","unresolved":false,"context_lines":[{"line_number":65,"context_line":"         available resource classes such as C(Server) inside C(*.py) files.\""},{"line_number":66,"context_line":"    required: true"},{"line_number":67,"context_line":"    type: str"},{"line_number":68,"context_line":"  updateable_attributes:"},{"line_number":69,"context_line":"    description:"},{"line_number":70,"context_line":"      - List of attribute names which can be updated."},{"line_number":71,"context_line":"      - When I(updateable_attributes) is not specified, then all attributes"}],"source_content_type":"text/x-python","patch_set":16,"id":"c6cc855d_a045631d","line":68,"in_reply_to":"cb438a48_5d8e9d07","updated":"2023-01-26 08:35:19.000000000","message":"Done","commit_id":"ef4652f226be351a2555e7fc2a235c6a35e917a5"},{"author":{"_account_id":27900,"name":"Artem Goncharov","email":"artem.goncharov@gmail.com","username":"gtema"},"change_message_id":"edf007e36bde421aca392ffc6658adaae93bef0e","unresolved":true,"context_lines":[{"line_number":189,"context_line":"        network_id: \"{{ network_external.resource.id }}\""},{"line_number":190,"context_line":"    wait: true"},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"- name: Attach router to internal subnet"},{"line_number":193,"context_line":"  openstack.cloud.router:"},{"line_number":194,"context_line":"    cloud: devstack-admin"},{"line_number":195,"context_line":"    name: ansible_router"}],"source_content_type":"text/x-python","patch_set":16,"id":"6b3100ea_2c78ba64","line":192,"updated":"2023-01-25 18:49:28.000000000","message":"does this example really belong here?","commit_id":"ef4652f226be351a2555e7fc2a235c6a35e917a5"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"c2a24d809c9a49dd8c3e0603bef1f5f65387cfce","unresolved":false,"context_lines":[{"line_number":189,"context_line":"        network_id: \"{{ network_external.resource.id }}\""},{"line_number":190,"context_line":"    wait: true"},{"line_number":191,"context_line":""},{"line_number":192,"context_line":"- name: Attach router to internal subnet"},{"line_number":193,"context_line":"  openstack.cloud.router:"},{"line_number":194,"context_line":"    cloud: devstack-admin"},{"line_number":195,"context_line":"    name: ansible_router"}],"source_content_type":"text/x-python","patch_set":16,"id":"208f70dc_902c81ad","line":192,"in_reply_to":"6b3100ea_2c78ba64","updated":"2023-01-25 19:44:16.000000000","message":"Same as in CI test.","commit_id":"ef4652f226be351a2555e7fc2a235c6a35e917a5"},{"author":{"_account_id":27900,"name":"Artem Goncharov","email":"artem.goncharov@gmail.com","username":"gtema"},"change_message_id":"edf007e36bde421aca392ffc6658adaae93bef0e","unresolved":true,"context_lines":[{"line_number":214,"context_line":"      port_id: \"{{ port_internal.resource.id }}\""},{"line_number":215,"context_line":"  register: ip"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"- name: List images"},{"line_number":218,"context_line":"  openstack.cloud.resources:"},{"line_number":219,"context_line":"    cloud: devstack-admin"},{"line_number":220,"context_line":"    service: image"}],"source_content_type":"text/x-python","patch_set":16,"id":"323c1b09_3dc96ca0","line":217,"updated":"2023-01-25 18:49:28.000000000","message":"I would also drop all (or at least majority) other resources example from examples here. It produces huge doc page with lot of irrelevant information that may be overseen (outdated)","commit_id":"ef4652f226be351a2555e7fc2a235c6a35e917a5"},{"author":{"_account_id":32962,"name":"Jakob Meng","email":"code@jakobmeng.de","username":"jakobmeng@web.de"},"change_message_id":"c2a24d809c9a49dd8c3e0603bef1f5f65387cfce","unresolved":true,"context_lines":[{"line_number":214,"context_line":"      port_id: \"{{ port_internal.resource.id }}\""},{"line_number":215,"context_line":"  register: ip"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"- name: List images"},{"line_number":218,"context_line":"  openstack.cloud.resources:"},{"line_number":219,"context_line":"    cloud: devstack-admin"},{"line_number":220,"context_line":"    service: image"}],"source_content_type":"text/x-python","patch_set":16,"id":"7ac85b2a_84297267","line":217,"in_reply_to":"323c1b09_3dc96ca0","updated":"2023-01-25 19:44:16.000000000","message":"The idea of this example section is to show\n\n(a) how different types of resources can be created and deleted with this module,\n(b) a complete and copy-paste-able example of how a public server can be created with this module.\n\nThe CI integration test is not useable as an example because it is very verbose and has scattered checks all over the place.\n\nWithout a proper alternative for examples in this collection, we have to fall back to EXAMPLES. In case this changes in future we can move the examples over there.","commit_id":"ef4652f226be351a2555e7fc2a235c6a35e917a5"},{"author":{"_account_id":27900,"name":"Artem Goncharov","email":"artem.goncharov@gmail.com","username":"gtema"},"change_message_id":"d25120428f16d89e876ff7ce720242815085dcfc","unresolved":false,"context_lines":[{"line_number":214,"context_line":"      port_id: \"{{ port_internal.resource.id }}\""},{"line_number":215,"context_line":"  register: ip"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"- name: List images"},{"line_number":218,"context_line":"  openstack.cloud.resources:"},{"line_number":219,"context_line":"    cloud: devstack-admin"},{"line_number":220,"context_line":"    service: image"}],"source_content_type":"text/x-python","patch_set":16,"id":"38e49f76_775a5d54","line":217,"in_reply_to":"7ac85b2a_84297267","updated":"2023-01-26 12:07:19.000000000","message":"Done","commit_id":"ef4652f226be351a2555e7fc2a235c6a35e917a5"}]}
