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

Add -config-directory argument to Windows service #2312

Closed
wants to merge 2 commits into from
Closed

Add -config-directory argument to Windows service #2312

wants to merge 2 commits into from

Conversation

cosmopetrich
Copy link

@cosmopetrich cosmopetrich commented Jan 24, 2017

Fixes #1797

Telegraf will fail to start if given a -config-directory that doesn't exist. The arguments used by the service won't change unless Telegraf is run with -service install and -service uninstall, so breakage as a result of this change seems unlikely.

Required for all PRs:

  • CHANGELOG.md updated (we recommend not updating this until the PR has been approved by a maintainer)
  • Sign CLA (if not already signed)
  • README.md updated (if adding a new plugin)

@sparrc
Copy link
Contributor

sparrc commented Jan 24, 2017

you need to run go fmt ./... on your code

@cosmopetrich
Copy link
Author

Sorry about that. Fixed.

@cosmopetrich
Copy link
Author

It looks like scripts/build.py doesn't create the telegraf.d directory, so that won't be included in the Windows .zip file. On Linux, scripts/post-install.sh takes care of creating that directory.

Is that something we should worry about here, or should it be left up to users installing the service on Windows?

@sparrc
Copy link
Contributor

sparrc commented Jan 24, 2017

can you modify build.py to make that directory?

@sparrc sparrc added this to the 1.3.0 milestone Jan 25, 2017
@sparrc
Copy link
Contributor

sparrc commented Feb 1, 2017

ping

Copy link
Contributor

@sparrc sparrc left a comment

Choose a reason for hiding this comment

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

build.py update

@cosmopetrich
Copy link
Author

Apologies @sparrc, I hadn't had a chance to revisit this yet.

It looks like it may have been superseded by #2330?

@cosmopetrich
Copy link
Author

Ah, no, that doesn't set a default.

@danielnelson danielnelson modified the milestones: 1.3.0, 1.4.0 Apr 20, 2017
@danielnelson danielnelson modified the milestones: 1.4.0, 1.5.0 Aug 14, 2017
@danielnelson danielnelson added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label Aug 24, 2017
@danielnelson danielnelson modified the milestones: 1.5.0, 1.6.0 Nov 30, 2017
@russorat russorat modified the milestones: 1.6.0, 1.7.0 Feb 1, 2018
@danielnelson
Copy link
Contributor

I added a bit of documentation around using the configuration directory in Windows as a service. a9afd2f

I don't think we should set a default value across the board, but if we add an installer in the future it could help setup a location and directory for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin platform/windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants