search

nkakouros-original / ansible-role-nextcloud

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

Simplify JSON parsing by disabling warnings #34

Closed simonspa closed 4 years ago

simonspa commented 4 years ago

I found the --no-warnings option for the occ command which gets rid of global warnings creating problems with JSON output. Enabling them allows us to simply use the from_json filter.

nkakouros commented 4 years ago

Based on some quick testing, I think that the issue with the warnings/errors being sent to stdout is fixed in later versions of nextcloud. If that is the case, we do not even need the --no-warnings option.

On my server, I get a warning about the PHP memory limit being too low when I run occ. But this warning gets sent to stderr. Do you have the same behavior?

Also, in any case, the set_fact tasks that read stdout from the command output are not needed as ansible will convert them automatically from json (provided they are valid json).

nkakouros commented 4 years ago

Also, this closes #29 .

simonspa commented 4 years ago

I tested this and I do get the same warning as you when setting a very low PHP memory limit like 15MB. Interestingly, the --no-warnings flag suppresses it for some commands (the relevants) but not for others:

app:list

# sudo -u www-data php occ app:list --output=json
The current PHP memory limit is below the recommended value of 512MB.
{"enabled":{"accessibility":"1.3.0"},"disabled":{"encryption":null}}
# sudo -u www-data php occ app:list --no-warnings --output=json
{"enabled":{"accessibility":"1.3.0"},"disabled":{"encryption":null}}

app:getpath

# sudo -u www-data php occ app:getpath apporder --no-warnings --output=json
The current PHP memory limit is below the recommended value of 512MB.
/var/www/nextcloud/apps/apporder

Since we only parse JSON output from app:list I think it is fine to go with this solution.

simonspa commented 4 years ago

Using the output directly doesn't work since it is a string, encapsulated in stdout but we need dictionaries. The set_fact tasks seem to be the simplest solution to get them.

simonspa commented 4 years ago

There is one odd corner case left with apps: if I try to install an app that exists but isn't available for the current server version (like currently is the case for issuetemplate) the app is downloaded and unzipped but the install command will fail anyway. The app thereafter is seen as installed but cannot be enabled (same error is thrown):

# sudo -u www-data php occ app:enable issuetemplate
App "Issue Template" cannot be installed because it is not compatible with this version of the server.

Not sure how to deal with this situation properly...

nkakouros commented 4 years ago

Is this the default.behavior of the occ tool? If yes, then it is OK from our side.

simonspa commented 4 years ago

Yes, it is the default behavior of occ

nkakouros commented 4 years ago

Thanks for working on this as well! I think I should make a new release for Nextcloud version 17.

simonspa commented 4 years ago

Sounds like a good idea! I'd still like to look into the upgrade process, I have problems upgrading from 17.0.0 to 17.0.1 with the role and would like to understand why. I'll probably have time the coming days...