nkakouros-original / ansible-role-nextcloud

An Ansible role to install Nextcloud
GNU General Public License v3.0
11 stars 5 forks source link

Read version string from JSON object of occ status #28

Closed simonspa closed 5 years ago

simonspa commented 5 years ago

I have found a way to completely circumvent the regex - we can read the clean versionstring from the JSON output of occ status.

Is there a reason to not use the from_json filter in the other cases where occ output is parsed? You added a comment about upgrade notices breaking it - does it only break the filter or also your manual json parsing?

nkakouros commented 5 years ago

Good idea, I don't remember if there was a reason back then to not use occ status or if I simply didn't notice it. See my comments.

simonspa commented 5 years ago

Okay, implemented. How about changing the other json conversions to from_json filters too?

nkakouros commented 5 years ago

What others do you think need to change?

simonspa commented 5 years ago

There are a few we could simplify in a similar way:

   become_user: "{{ nextcloud_file_owner }}"
-  register: nextcloud_installed_apps
+  register: _result
   changed_when: false

 - name: Remove non-json text from command output
   set_fact:
-    nextcloud_installed_apps: >-
-      {{
-        nextcloud_installed_apps.stdout[
-          (nextcloud_installed_apps.stdout.find('{')):
-        ]
-      }}
+    nextcloud_installed_apps: "{{ _result.stdout | from_json }}"
nkakouros commented 5 years ago

Oh I see. There was an issue with occ that would pollute the json output with warnings, etc. I do not know if this is still an issue. Could you open a new issue about this where we can keep track of it?

simonspa commented 5 years ago

Done so! #29

nkakouros commented 5 years ago

Great! I 'll try to have the tests cover also the use case in #29 to be sure any changes do not break the role.