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

Add pcie_errors plugin #911

Merged
merged 3 commits into from Feb 18, 2020
Merged

Conversation

prabiegx
Copy link
Contributor

@prabiegx prabiegx commented Feb 13, 2020

The pcie_errors plugin collects PCI Express errors from Device Status in Capability structure and from Advanced Error Reporting Extended Capability where available.

Optional[String] $source = 'sysfs',
Optional[String] $access_dir = undef,
Optional[Boolean] $report_masked = false,
Optional[Boolean] $persistent_notifications = false,
Copy link
Member

Choose a reason for hiding this comment

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

does Optional make sense here? In most cases undef isn't a useful value for a Boolean variable.

@bastelfreak bastelfreak added enhancement New feature or request needs-work not ready to merge just yet labels Feb 16, 2020
#
class collectd::plugin::pcie_errors (
Enum['present', 'absent'] $ensure = 'present',
Optional[String] $source = 'sysfs',
Copy link
Member

Choose a reason for hiding this comment

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

We are never able to set $source to undef if it has a default value. We can only change sysfs to another string. Are sysfs and proc the only valid values? In that case we could use an enum here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Values other than sysfs and proc are treated as error. As suggested I changed data type to enum and updated test case.

@prabiegx
Copy link
Contributor Author

prabiegx commented Feb 18, 2020

It looks like build failed due to one of the configured repositories failing, which seems to be "random" issue not related to this change. @bastelfreak could failing job be repeated?

@bastelfreak bastelfreak removed the needs-work not ready to merge just yet label Feb 18, 2020
@bastelfreak bastelfreak merged commit 176618d into voxpupuli:master Feb 18, 2020
@prabiegx prabiegx deleted the collectd_pcie_errors branch February 20, 2020 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants