Skip to content
This repository has been archived by the owner on Nov 8, 2022. It is now read-only.

Fix #1499 update explaination to auto-discovery plugin. #1510

Merged
merged 1 commit into from
Feb 9, 2017

Conversation

nanliu
Copy link
Contributor

@nanliu nanliu commented Feb 7, 2017

No description provided.

@nanliu
Copy link
Contributor Author

nanliu commented Feb 7, 2017

This is related to: https://github.com/intelsdi-x/snap-packaging/pull/34. There are changes to the packaging repo config file, so we take advantage of new settings such as max_plugin_restarts. If the we decide to change the comments in the config file, I will update the packaging repo to match the updates here.

# of the Snap daemon
auto_discover_path: /opt/snap/plugins
# auto_discover_path sets the directory(s) to auto load plugins and tasks on
# the start of the snap daemon. This can be a comma separated list of directories.
Copy link
Contributor

Choose a reason for hiding this comment

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

giving an example of a comma separated list?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The example below is two directories, one for plugins, one for tasks.

Copy link
Contributor

Choose a reason for hiding this comment

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

oh, it does not use command instead of ":"?

Copy link
Contributor

@mbbroberg mbbroberg left a comment

Choose a reason for hiding this comment

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

Edits lgtm.

@nanliu
Copy link
Contributor Author

nanliu commented Feb 7, 2017

Since the packaging repo isn't public, I just want to check and make sure the follow changes are sensible defaults for snapteld.conf.

auto_discover_path: /opt/snap/plugins:/opt/snap/tasks
max_plugin_restarts: -1

I've also created a PR for snap-docker, but left out max_plugin_restarts because that is not compatible with older versions of snap, which will prevent the containers from starting if we ci older versions such as: SNAP_VERSION=0.19.0.

cc: @intelsdi-x/snap-maintainers

@jcooklin
Copy link
Collaborator

jcooklin commented Feb 8, 2017

I would be more conservative with max_plugin_restarts. If there is a problem with the system or a plugin I don't think we should continually restart the plugin by default.

@candysmurf
Copy link
Contributor

LGTM

@nanliu
Copy link
Contributor Author

nanliu commented Feb 9, 2017

I'll switch max_plugin_restarts back to the default for the package to keep everything consistent.

@nanliu nanliu merged commit a429f3e into intelsdi-x:master Feb 9, 2017
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.

4 participants