cloud-py-api / cloud_py_api

Nextcloud framework for developing apps using Python
https://apps.nextcloud.com/apps/cloud_py_api
GNU Affero General Public License v3.0
50 stars 7 forks source link

No DB connection because occ config:system:get doesn't always return a proper value #46

Closed DrMurx closed 1 year ago

DrMurx commented 1 year ago

Describe the bug

If Nextcloud's database config is done via environment variables (possible with variables named NC_<cfgvalue>, see PR 3966), occ config:system:get does not respect this and cloud_py_api can't connect to the database.

I opened a Nextcloud bug on this subject because I think the occ command should reliably return the configuration, but one could still argue that occ config:system:get should only complement the functionality of occ config:system:set and only operate on the config.php. Therefore you might want to explore other options on your side.

To Reproduce

See instructions in https://github.com/nextcloud/server/issues/36126

Expected behavior

Working database connection, regardless of how it is configured.

bigcat88 commented 1 year ago

Thanks @DrMurx for pointing on this, good catch!

DrMurx commented 1 year ago

Given that a config via environment variable is mainly used in container environments, there is another option to solve this. Doing things the "docker way" means to run dedicated containers for specific services.

So instead of spawning the MediaDC instance (or any other cloud_py_api based service) from mod_php or php-fpm, it could run in a dedicated container.

There's a similar discussion around Nextcloud's very own notify_push in https://github.com/nextcloud/docker/issues/1422 with a simple container solution described in this comment.

Note that notify_push parses the config.php directly with a complicated PHP parsing logic in Go (🤦 ), so it neither accepts the NC_ variables nor does it respect additional config files. But it understands dedicated variables like DATABASE_URL, DATABASE_PREFIX and REDIS_URL.

bigcat88 commented 1 year ago

@DrMurx if I understand right, that is what is needed

DrMurx commented 1 year ago

Yes, PR #47 should resolve the current issue.

I'd still suggest to make projects that base on cloud_py_api docker-friendly. Should I open a new issue for such a feature?

bigcat88 commented 1 year ago

Yes, it's better to do it in the new issue.