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

Remove possible "ignore" value from "use_node_name" #1140

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

sumpfralle
Copy link
Collaborator

The values yes and ignore for use_node_name have the same effect.

The ignore value was never documented anywhere.

@sumpfralle
Copy link
Collaborator Author

This change set is based upon #1139.

@coveralls
Copy link

coveralls commented Dec 15, 2018

Pull Request Test Coverage Report for Build 2715

  • 5 of 8 (62.5%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 73.011%

Changes Missing Coverage Covered Lines Changed/Added Lines %
lib/Munin/Master/Node.pm 5 8 62.5%
Totals Coverage Status
Change from base Build 2714: 0.02%
Covered Lines: 1028
Relevant Lines: 1408

💛 - Coveralls

@steveschnepp
Copy link
Member

This is more to cope with none standard nodes that exist out there.

@sumpfralle
Copy link
Collaborator Author

This is more to cope with none standard nodes that exist out there.

You mean: implementations of munin-node that cannot handle the node_name argument of list?
In this case the above change helps here, too: the node_name is not added as an argument anymore, if it matches the name announced by the node.

@sumpfralle
Copy link
Collaborator Author

@steveschnepp: do you see any issues with the removal of this option?

@steveschnepp steveschnepp added this to the 2.999.13 milestone Jun 9, 2019
The setting was introduced in 6adf38a.

But somehow it partially lost its meaning in the merge commit
c8228b8 (Merge branch 'sql' into devel).
Afterwards it was only used for for breaking the node communication
in case of a missing "host" attribute (which can never be empty as
it is an integral part of the node specification in the master
configuration).  Thus it is not useful anymore.

The purpose of this setting was later replaced with
"use_node_name ignore" in dd4cbb5.
The additional value "ignore" was added in dd4cbb5.

It caused the following "list" commands to be sent to the node:
* "yes" -> "list $self->{node_name}"
* "ignore" -> "list "
* "no" -> "list $self->{host}"
* any other value -> "list $self->{host}"

The above "$self->{node_name}" is the name advertised by the node during
the opening of the connection.
"$self->{host}" is the name of the node section in the master
configuration.

The new behaviour is the following:
* "yes" -> "list"
* "ignore" -> "list"
* "no" -> "list $self->{host}"
* any other value -> "list $self->{host}"

This behaviour has the same effect as before, as the request for "list"
(without a specific node name) is handley by munin-node exactly, as if
its "node_name" is supplied.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants