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 new filtering options, continue deprecations #2483

Merged
merged 3 commits into from
Oct 30, 2018
Merged

Add new filtering options, continue deprecations #2483

merged 3 commits into from
Oct 30, 2018

Conversation

ofek
Copy link
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

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
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
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
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
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

Copy link
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 👌

Copy link
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