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

Fix return value or loglevel for several plugins #3182

Merged
merged 4 commits into from Jun 28, 2019

Conversation

faxm0dem
Copy link
Contributor

@faxm0dem faxm0dem commented Jun 19, 2019

Some plugins' RC was != 0 and started to fail due to stricter verification introduced by 3b9c7b2.
This commit fixes those return values. For some plugins, fix verbosity of error message: non-zero rc
should be error, not warning.

Fixes #3180

What I chose to do:

  • fix RC when I judged it to be non-fatal (load, logfile)
  • fix loglevel from WARNING to ERROR in other cases (barometer, openvpn, ted)
  • add log message (syslog)

ChangeLog: fix incorrect return codes or log level for several plugin config callbacks

@faxm0dem faxm0dem force-pushed the f/config-return-codes branch 5 times, most recently from bc37a00 to 742f1db Compare June 19, 2019 10:41
@mrunge mrunge self-assigned this Jun 19, 2019
src/daemon/configfile.c Outdated Show resolved Hide resolved
@mrunge
Copy link
Member

mrunge commented Jun 21, 2019

Can you please re-run clang-format?

3b9c7b2 introduced stricter RC check.
This commit provides user feedback when config callback is failing.

Change-Id: Ia9c13048e95559b5be84477fc1602ff418a1df37
@faxm0dem
Copy link
Contributor Author

I reran the clang lint, everything ok now? I can squash everything

@rpv-tomsk
Copy link
Contributor

As you can see, that is not ok and CI still failing.
Use contrib/format.sh as proposed by CI.

@faxm0dem
Copy link
Contributor Author

sorry about that - should be okay now

rpv-tomsk
rpv-tomsk previously approved these changes Jun 27, 2019
@rpv-tomsk rpv-tomsk added Automerge Labels PRs to be merged by a bot once approved Bug A genuine bug labels Jun 27, 2019
@rpv-tomsk
Copy link
Contributor

rpv-tomsk commented Jun 27, 2019

Further reading:

https://github.com/orgs/collectd/teams/trusted-contributors/discussions/1

#2958

I do not understand why Team prevents other people from project developing.
Can anybody explain me?

Fᴀʙɪᴇɴ Wᴇʀɴʟɪ added 3 commits June 28, 2019 11:03
Some plugins' RC was != 0 and started to fail due to
stricter verification introduced by 3b9c7b2.
This commit fixes those return values.
For some plugins, fix verbosity of error message: non-zero rc
should be error, not warning.

Change-Id: I9a3f1f80e266858b6744fd9d9d99b352b8d94306

Change-Id: Ibf6ebc6cdc93c6e105d488e4a131dcb6e8eea19b

Change-Id: I35bac15fa0a89b068575739ac1cff0115c9d3a40

s

Change-Id: I992002c56763fbdea5347e5b6e176cc86f5a08ce
Change-Id: Iae7c3208024372485fd0901898cbe2e178610082
@faxm0dem faxm0dem changed the title WIP: fix config return codes fix config return codes Jun 28, 2019
@faxm0dem faxm0dem changed the title fix config return codes Fix return value or loglevel for several plugins Jun 28, 2019
Copy link
Member

@mrunge mrunge left a comment

Choose a reason for hiding this comment

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

Thank you for your work on this!

@mrunge
Copy link
Member

mrunge commented Jun 28, 2019

Hmm, automerge doesn't work?

@rpv-tomsk
Copy link
Contributor

oops ? :-))) unexpectedly, right? :-))))

@rpv-tomsk
Copy link
Contributor

Team prevents other people from project developing.

Live with it and enjoy.

@mrunge
Copy link
Member

mrunge commented Jun 28, 2019

So, automerge issues have nothing to do with this patch. I'll merge it now manually. Let's take the discussion from here.
I see your point @rpv-tomsk , let's see what we can do to fix this.

@mrunge mrunge merged commit 0d72e6a into collectd:master Jun 28, 2019
@@ -56,7 +57,8 @@ static int sl_config(const char *key, const char *value) {
} else if (strcasecmp(key, "NotifyLevel") == 0) {
notif_severity = parse_notif_severity(value);
if (notif_severity < 0)
Copy link
Member

Choose a reason for hiding this comment

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

Missing brackets for if? "Return 1" is outside of if scope.

faxm0dem pushed a commit to ccin2p3/collectd that referenced this pull request Jul 2, 2019
Change-Id: I0972f74f3fff05cf29fa9b0be383f0b0df1e6d03
Fixes: collectd#3200
rpv-tomsk added a commit that referenced this pull request Jul 2, 2019
@octo octo added Fix A pull request fixing a bug and removed Bug A genuine bug labels Jan 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Automerge Labels PRs to be merged by a bot once approved Fix A pull request fixing a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

plugin load fails to parse ReportRelative config option
5 participants