Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nixos/nextcloud: Add services.nextcloud.autoUpdateApps #61717

Merged
merged 1 commit into from May 21, 2019
Merged

nixos/nextcloud: Add services.nextcloud.autoUpdateApps #61717

merged 1 commit into from May 21, 2019

Conversation

maralorn
Copy link
Member

Motivation for this change

This change adds the options services.nextcloud.autoUpdateApps.enable and services.nextcloud.autoUpdateApps.startAt.

If set it configures a systemd-timer unit to run occ app:update --allat the specified time.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@flokli
Copy link
Contributor

flokli commented May 20, 2019

@maralorn Thanks for the PR! Could you add a paragraph to nixos/modules/services/web-apps/nextcloud.xml explaining apps are currently installed imperatively, and link to the options introduced in this PR?

Maybe also enable it in the tests, so we hit the code paths there (even though we obviously can't test timers get fired)

@maralorn
Copy link
Member Author

@flokli There you go!

Copy link
Contributor

@flokli flokli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the followup!

nixos/modules/services/web-apps/nextcloud.xml Outdated Show resolved Hide resolved
nixos/modules/services/web-apps/nextcloud.nix Outdated Show resolved Hide resolved
Copy link
Contributor

@flokli flokli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGMT :-)

Can you squash this together?

Copy link
Contributor

@flokli flokli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, I missed that part - references only exist on leaf nixos options, not all nodes. Sorry for the noise!

nixos/modules/services/web-apps/nextcloud.xml Outdated Show resolved Hide resolved
nixos/nextcloud: Add documentation for nextcloud app installation and updates

nixos/nextcloud: Enable autoUpdateApps in nextcloud test

nixos/nextcloud: Fix typo in nixos/modules/services/web-apps/nextcloud.xml

Co-Authored-By: Florian Klink <flokli@flokli.de>

nixos/nextcloud: Escape html in option description

nixos/nextcloud: Fix autoUpdateApps URL in documentation.

Co-Authored-By: Florian Klink <flokli@flokli.de>
@flokli
Copy link
Contributor

flokli commented May 21, 2019

Thanks a lot!

@flokli flokli merged commit 36dfdcc into NixOS:master May 21, 2019
@maralorn
Copy link
Member Author

Can we backport this to 19.03? It's just a new option. That shouldn't break anything.

@flokli
Copy link
Contributor

flokli commented May 21, 2019

backported to 19.03 in 246ebba.

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

Successfully merging this pull request may close these issues.

None yet

2 participants