Skip to content

feat(daemon): add --show-deploy-config flag to daemon:list command#791

Open
isaaclee12 wants to merge 1 commit intonextcloud:mainfrom
isaaclee12:feat/785/add-deploy-config-info
Open

feat(daemon): add --show-deploy-config flag to daemon:list command#791
isaaclee12 wants to merge 1 commit intonextcloud:mainfrom
isaaclee12:feat/785/add-deploy-config-info

Conversation

@isaaclee12
Copy link

@isaaclee12 isaaclee12 commented Feb 19, 2026

Relevant Issue: #785

This is my first PR to Nextcloud!

NOTES:

  • The force push below is due to the DCO check failing. I ran git commit --amend -s and git push --force-with-lease origin feat/785/add-deploy-config-info since there's only one commit from me.

  • I have yet to set up nextcloud-docker-dev on my machine, so I haven't been able run tests on this yet. I'm hoping I can see the results in GitHub Actions once those workflows are kicked off.

Signed-off-by: Isaac Lee <isaac.wonha.lee@outlook.com>
@oleksandr-nc
Copy link
Contributor

Hey Isaac, thanks for the PR and welcome to Nextcloud contributions!

The idea behind this is solid and matches what was requested in the issue. I have a few things that need to be changed before we can merge this though.

First, please revert all the code style changes. The brace placement in our project is same-line (opening brace on the same line as the class/method declaration), and your PR switches several of them to next-line style. Same thing with the constructor parameter alignment change. Just keep the existing style as-is and only touch what is needed for the feature.

Second, the deploy config output is incomplete. You are only outputting net, nextcloud_url, computeDevice and optionally harp, but the config can also contain haproxy_password, kubernetes settings, resourceLimits, registries, and additional_options.
Instead of cherry-picking fields, just output the whole deploy config as-is. That way it stays future-proof and we don't need to update this code every time a new field is added. Something like json_encode($deployConfig, JSON_UNESCAPED_SLASHES) on the whole thing.
You probably want to skip haproxy_password from the output though since that is a secret.

Third, putting JSON_PRETTY_PRINT inside a table cell is going to look really bad in the terminal. The table already has 10+ columns and a multi-line JSON blob in one cell will break the layout. I would suggest one of two approaches: either use compact (single-line) JSON in the table column, or better yet, when the flag is passed, skip the table entirely and just output each daemon as a formatted JSON block. The second option would actually be more readable and useful for people who want to inspect the full config.

Also a small PHP style thing: use $rows[] = $newRow instead of array_push($rows, $newRow). That is the idiomatic way in PHP and it is what the rest of the file uses.

And last, please make sure to test this locally before resubmitting. Setting up nextcloud-docker-dev takes a bit of time but it is worth it to catch issues early. For example, some deploy configs from older setups might not have all keys present (like computeDevice or net), and accessing them without a fallback would throw PHP notices.

Thanks again for picking this up. The feature is definitely wanted, just needs some adjustments. Let me know if you have any questions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments