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

Taskserver not updating it's config #22705

Closed
pstn opened this issue Feb 12, 2017 · 6 comments
Closed

Taskserver not updating it's config #22705

pstn opened this issue Feb 12, 2017 · 6 comments

Comments

@pstn
Copy link
Contributor

pstn commented Feb 12, 2017

Right now a running taskserver isn't affected by a nixos config change because the actual config file, taskserver uses only gets initialized with an include of the internal configuration file.
Also this leads to breakage when nix-store garbage collects this file because there might be serveral newer versions that get ignored.
My proposal is to always symlink the newest config file to the data folder to fix this issue.

//cc @matthiasbeyer and @aszlig

@aszlig
Copy link
Member

aszlig commented Feb 12, 2017

Hm, this has the implication that it requires manual intervention by people that have already deployed Taskserver in that they need to replace the include path of the config file.

I'd suggest adding a new option to taskd server, like -c <configfile> and use it to refer to the store path directly. We can still use include within our own generated configuration file to refer to the one within the dataDir.

@aszlig
Copy link
Member

aszlig commented Feb 12, 2017

Another method would be to use overrides: https://git.tasktools.org/projects/TM/repos/taskd/browse/src/api.cpp#82-103

@aszlig
Copy link
Member

aszlig commented Feb 12, 2017

Okay, the config file is expected to be mutable, so I'd suggest the latter and instead of creating a config file which is simply included, let's shovel all configuration items into the command line, what do you think?

However, this still leads to some breakage in existing deployments if the config file refers to a store path that doesn't exist anymore and I'd like to avoid weird fixups like:

sed -i -e '\!^include ${builtins.storeDir}!d' "${cfg.dataDir}/config"

@aszlig
Copy link
Member

aszlig commented Feb 12, 2017

Btw: Just symlinking the config file doesn't help as well, because if Taskserver writes its configuration file it strips out all include directives and it only consists of key/value pairs. So this is a problem in our current implementation as well, because the include directive only gets inserted once on init.

@pstn
Copy link
Contributor Author

pstn commented Feb 12, 2017

I'm fine with adding the options to the cli invocation. Seems to be the best solution. It could also break running systems when people have changed their config, but since I'm the first person to notice this behavior I don't think that many people are using taskserver right now. Maybe we could include a warning output when a config file is found?

@matthiasbeyer
Copy link
Contributor

@aszlig you are, indeed, awesome! Great commit message, thanks a lot!

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

No branches or pull requests

3 participants