Skip to content

Add new filtering options, continue deprecations#2483

Merged
ofek merged 3 commits intomasterfrom
ofek/disk
Oct 30, 2018
Merged

Add new filtering options, continue deprecations#2483
ofek merged 3 commits intomasterfrom
ofek/disk

Conversation

@ofek
Copy link
Copy Markdown
Contributor

@ofek ofek commented Oct 29, 2018

What does this PR do?

Adds new partition filtering options

  • file_system_whitelist
  • file_system_blacklist
  • device_whitelist
  • device_blacklist
  • mount_point_whitelist
  • mount_point_blacklist

And began deprecation of

  • excluded_filesystems
  • excluded_disks
  • excluded_disk_re
  • excluded_mountpoint_re

Motivation

  • Customer requests for ability to white list
  • Less confusing options: disk vs device, duplicate options, options of different types, etc.

Additional Notes

Also converted config to new style

@codecov-io
Copy link
Copy Markdown

codecov-io commented Oct 29, 2018

Codecov Report

Merging #2483 into master will increase coverage by 9.01%.
The diff coverage is 97.92%.

@@            Coverage Diff             @@
##           master    #2483      +/-   ##
==========================================
+ Coverage   84.52%   93.54%   +9.01%     
==========================================
  Files         622       11     -611     
  Lines       35060      558   -34502     
  Branches     4270       66    -4204     
==========================================
- Hits        29636      522   -29114     
+ Misses       4137       28    -4109     
+ Partials     1287        8    -1279

Copy link
Copy Markdown
Contributor

@zippolyte zippolyte left a comment

Choose a reason for hiding this comment

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

Great job ! Just a few minor comments

valid_patterns.append(pattern)

if valid_patterns:
return re.compile('|'.join(valid_patterns), casing)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we need to make sure that valid_patterns are enclosed in parenthesis before joining them. Otherwise a pattern containing | could break the matching no ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't think so as a|b|c == (a|b)|c, right?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah I thought I had a example but was mistaken, you're probably right

Comment thread disk/datadog_checks/disk/disk.py Outdated
Comment thread disk/datadog_checks/disk/disk.py Outdated
Comment thread disk/datadog_checks/disk/disk.py Outdated
Comment thread disk/datadog_checks/disk/disk.py Outdated
Copy link
Copy Markdown
Contributor

@zippolyte zippolyte left a comment

Choose a reason for hiding this comment

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

Just a few more function names comments and it will be 👌

Comment thread disk/datadog_checks/disk/disk.py Outdated
Copy link
Copy Markdown
Contributor

@zippolyte zippolyte left a comment

Choose a reason for hiding this comment

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

Thanks, looks good now 💯

@ofek ofek merged commit 7498ade into master Oct 30, 2018
@ofek ofek deleted the ofek/disk branch October 30, 2018 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants