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 dbi query.conf.erg template #960

Merged
merged 1 commit into from Oct 26, 2020
Merged

Conversation

VtG242
Copy link
Contributor

@VtG242 VtG242 commented Oct 22, 2020

Pull Request (PR) description

Without this change collectd refuse to load configuration for dbi plugin with error (at least for me - collectd-5.8.1):
The Query block needs exactly one string argument.
dbi plugin: No blocks have been found. Without them, this plugin can't do anything useful, so we will return an error.

Without this collectd refuse to load configuration with error:
The Query block needs exactly one string argument.
dbi plugin: No <Query> blocks have been found. Without them, this plugin can't do anything useful, so we will return an error.
@bastelfreak bastelfreak added bug Something isn't working needs-tests labels Oct 23, 2020
@bastelfreak
Copy link
Member

@VtG242 thanks for the patch! Any chance to write a simple acceptance test that valides the new config?

@VtG242
Copy link
Contributor Author

VtG242 commented Oct 23, 2020

To be honest I don't have much experience with this.
If there is some documentation how to do it I could try ;-)

@bastelfreak
Copy link
Member

Hey,
can you take a look at the different test files in https://github.com/voxpupuli/puppet-collectd/tree/master/spec/acceptance ?
for each of them we spin up multiple docker containers with centos/debian/ubuntu, apply the puppet code from the test file and afterwards the checks. there are already a few files that test collectd plugins. We had multiple quoting issues in the past and it's always bad when we change it back and forth. So if we have a very simple test that just checks if collectd successfully loaded the config file, that would already be sufficient. you can copy one of the existing files, adjust it for the dbi plugin and commit it. Our CI setup will automatically test if.

If you've any further questions, don't hesitate to ping us in the IRC channel #voxpupuli on freenode or the slack room #voxpupuli on https://slack.puppet.com/

@VtG242
Copy link
Contributor Author

VtG242 commented Oct 23, 2020

Ok I will take a look at it once I will have a little spare time.

@VtG242
Copy link
Contributor Author

VtG242 commented Oct 26, 2020

I could write test, but I need support within docker images. For real test that collectd is running with dbi plugin enabled I need any db engine there (e.g. sqlite3 for simplicity) with some simple database and table with data e.g.:
sqlite3 /opt/dbi-test.db 'CREATE TABLE TEST(ID INT);INSERT INTO TEST (ID) VALUES ('1');'
Is it possible?

@bastelfreak
Copy link
Member

it's a bit annoying that collect not simply enables the plugin :( We could setup databases during the tests, but that's probably not worth the hassle for this tiny fix.

@bastelfreak bastelfreak merged commit 3ba5fa1 into voxpupuli:master Oct 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants