search

nextcloud / ansible-collection-nextcloud-admin

The ansible galaxy for your nextcloud administrative needs.
https://galaxy.ansible.com/nextcloud/admin
BSD 2-Clause "Simplified" License
138 stars 77 forks source link

A new role, which downloads the archive to the control host. This all… #356

Open marioqxx opened 4 months ago

marioqxx commented 4 months ago

…ows then to use the install-feature, that downloaded archives are transferred to the remote-host instead of downloading. This is the intial attempt. Therefore, please review and provide your comments.

aalaesar commented 1 month ago

hello there @marioqxx . Thank you for still contributing to the collection. I have reviewed the role and it's already well done,documented and integrated with the same variables as install_nextcloud.

There is definitively integration to do with it in the main role already. could you kindly add a molecule test for the role ?

During reading It came to my mind some questions:

regards. Aal

aalaesar commented 1 month ago

also please rebase the branche from main: the CI has been changed

marioqxx commented 1 month ago

@aalaesar,

thanks for looking into this! I have some issue which I need to resolve in order make my development environment work again, so please be a bit patient. I have accepted your proposed change. Good to know, this is also included in the nextcloud_install/nc_apps.yml I think. I will test your suggestion, because sometimes I have experienced issues with ansible, packing such expressions in a "var"-statement. But this again goes back to my insufficient knowledge of ansible.

To your question: "could it be splitted into 2 roles (one for download server, the other one for apps) ?" Of course. The way I have handled this is by means of the two variables: nextcloud_download_server: true nextcloud_download_apps: true So dependent on their setting you can download either server or app or both.

can we call it from install_nextcloud to avoid dual computation of the download links ? I don't think this would work. I had to create the computation of the download-link completely new and could not use anything from install_nextcloud. The way it is done is quite comprehensive, it includes HTML-parsing of the App-Shop WEB-page.

can we improve the version_major and version_full vars to merge then into one ? (using version filter instead of using tasks definitions ) You are right. I went the simplified way down the road, with an alignment to install_nextcloud. I'll have a look at it and see how this could be done better.

Once I have re-established my dev-setup, I'll potentially ask for your help regarding adding "molecule test". This is definitely outside my competence.