Skip to content
This repository has been archived by the owner on Mar 6, 2023. It is now read-only.

Add support for textfile collector #42

Merged
merged 1 commit into from
Jun 10, 2018
Merged

Add support for textfile collector #42

merged 1 commit into from
Jun 10, 2018

Conversation

SuperQ
Copy link
Collaborator

@SuperQ SuperQ commented Jun 10, 2018

[feature]

Create a default textfile collector directory and enable it by default.

@SuperQ SuperQ requested a review from paulfantom June 10, 2018 15:50
@SuperQ SuperQ force-pushed the superq/textfile branch 3 times, most recently from dd54aef to eb43a65 Compare June 10, 2018 16:42
@@ -2,8 +2,12 @@
node_exporter_version: 0.16.0
node_exporter_web_listen_address: "0.0.0.0:9100"

node_exporter_textfile_dir: "/opt/node_exporter/textfile"
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't it be somewhere in /var? Maybe in /var/local?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'd say '/var/lib`, how about that?

Copy link
Member

Choose a reason for hiding this comment

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

LGTM 👍

tasks/main.yml Outdated
group: "node-exp"
recurse: true
mode: 0755
when: node_exporter_textfile_dir is defined
Copy link
Member

Choose a reason for hiding this comment

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

It will always be defined since it is set in defaults/main.yml. Better to check if it is not empty (node_exporter_textfile_dir != "").

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sounds good.

[feature]

Create a default textfile collector directory and enable it by default.
@SuperQ
Copy link
Collaborator Author

SuperQ commented Jun 10, 2018

@paulfantom Fixed up, PTAL.

Copy link
Member

@paulfantom paulfantom left a comment

Choose a reason for hiding this comment

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

LGTM

@SuperQ SuperQ merged commit 0373b75 into master Jun 10, 2018
@SuperQ SuperQ deleted the superq/textfile branch June 10, 2018 21:23
@lock
Copy link

lock bot commented Mar 24, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Mar 24, 2019
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants