Skip to content

Comments

flexget service: create service module - backport to 16.03#14262

Merged
joachifm merged 1 commit intoNixOS:release-16.03from
peterhoeg:flexget
Apr 13, 2016
Merged

flexget service: create service module - backport to 16.03#14262
joachifm merged 1 commit intoNixOS:release-16.03from
peterhoeg:flexget

Conversation

@peterhoeg
Copy link
Member

@peterhoeg peterhoeg commented Mar 28, 2016

This module adds support for defining a flexget service.

Due to flexget insisting on being able to write all over where it finds its configuration file, we use a ExecStartPre hook to copy the generated configuration file into place under the user's home.

It's fairly ugly and I'm very open to suggestions.

Things done:
  • Tested using sandboxing (nix-build --option build-use-chroot true or nix.useChroot on NixOS)
  • Built on platform(s)
    • NixOS
    • OS X
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

flexget nixos module: run the service and specify configuration
d80126b 
This module adds support for defining a flexget service.

Due to flexget insisting on being able to write all over where it finds
its configuration file, we use a ExecStartPre hook to copy the generated
configuration file into place under the user's home. It's fairly ugly
and I'm very open to suggestions
@mention-bot
Copy link

mention-bot commented Mar 28, 2016

By analyzing the blame information on this pull request, we identified @edolstra, @bjornfor and @offlinehacker to be potential reviewers

joachifm
joachifm reviewed Mar 28, 2016
View reviewed changes
nixos/modules/services/torrent/flexget.nix
timerConfig = {
OnBootSec = "5m";
OnUnitInactiveSec = cfg.interval;
Unit = "flexget-runner.service";
Copy link
Contributor

@joachifm joachifm Mar 28, 2016

Choose a reason for hiding this comment

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

The timer Unit defaults to the service of the same name, so this is strictly unnecessary, but I guess it never hurts to be explicit

@joachifm
Copy link
Contributor

joachifm commented Mar 28, 2016

This looks good to me. Copying the configuration file is a sensible approach, IMO.

@joachifm joachifm added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (new) This PR adds a module in `nixos/` labels Mar 28, 2016
@joachifm
Copy link
Contributor

joachifm commented Mar 28, 2016

One alternative to copying into the homedir is to copy the configuration file into a volatile location, like RuntimeDirectory, but seeing as this runs under a non-login user anyway, it hardly makes a difference.

@peterhoeg peterhoeg mentioned this pull request Mar 29, 2016
Closed
3 tasks
@joachifm
Copy link
Contributor

joachifm commented Apr 13, 2016

Merging, given the lack of objections.

@joachifm joachifm merged commit c8b6c37 into NixOS:release-16.03 Apr 13, 2016
joachifm added a commit that referenced this pull request Apr 13, 2016
@joachifm
Revert "Merge pull request #14262 from peterhoeg/flexget"
027e666 
This reverts commit c8b6c37, reversing
changes made to 91a3e47.

Sorry, I did not notice that this was opened against the wrong branch ...
@joachifm
Copy link
Contributor

joachifm commented Apr 13, 2016

Ach, I realized this was opened against the wrong branch.

@joachifm
Copy link
Contributor

joachifm commented Apr 13, 2016

Please always use master or staging unless you're backporting something.

@peterhoeg
Copy link
Member Author

peterhoeg commented Apr 13, 2016

Sorry about. Do you need me to rebase?

@joachifm
Copy link
Contributor

joachifm commented Apr 13, 2016

Yeah, just rebase onto master and open a new PR.

@peterhoeg peterhoeg mentioned this pull request Apr 13, 2016
Merged
@peterhoeg peterhoeg changed the title flexget: create nixos service module flexget service: create service module - backport to 16.03 Apr 15, 2016
@peterhoeg
Copy link
Member Author

peterhoeg commented Apr 16, 2016

@joachifm , any chance we can get this into 16.03? Nothing depends on it so the impact should be limited.

@joachifm
Copy link
Contributor

joachifm commented Apr 16, 2016

Sounds reasonable to me. Please open a PR against release-16.03.

adrianpk added a commit to adrianpk/nixpkgs that referenced this pull request May 31, 2024
@adrianpk
Merge pull request NixOS#14262 from peterhoeg/flexget
fda246f 
flexget: create nixos service module
adrianpk added a commit to adrianpk/nixpkgs that referenced this pull request May 31, 2024
@adrianpk
Revert "Merge pull request NixOS#14262 from peterhoeg/flexget"
8c6dbcc 
This reverts commit c8b6c37, reversing
changes made to 91a3e47.

Sorry, I did not notice that this was opened against the wrong branch ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (new) This PR adds a module in `nixos/`

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@peterhoeg @mention-bot @joachifm