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.

Sign up for GitHub

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

Jump to bottom

Non systemd support with unittest #8785

Merged
merged 10 commits into from
Feb 26, 2021
Merged

Non systemd support with unittest #8785

merged 10 commits into from
Feb 26, 2021

Conversation

red15
Copy link
Contributor

@red15 red15 commented Feb 2, 2021 •
edited by sjwang90
Loading

Required for all PRs:

  • Associated README.md updated.
  • Has appropriate unit tests.

Fixes #3900

red15 added 2 commits January 29, 2021 19:28
@red15
Alter unit test method to use DiskInfoCache.udevDataPath
897b3a3 
Removing the udevPath global variable and it's dependencies.
This sets us up to allow better detection of udevDataPath to support
non-systemd linux versions.
@red15
Support older non-systemd udev data paths
ee30fe1 
Should properly solve (with minimal file probes) the udev mounting
problem.
telegraf-tiger[bot]
telegraf-tiger bot approved these changes Feb 2, 2021
View reviewed changes
Copy link
Contributor

@telegraf-tiger telegraf-tiger bot left a comment

Choose a reason for hiding this comment

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

🤝 ✅ CLA has been signed. Thank you!

@red15
Copy link
Contributor Author

red15 commented Feb 2, 2021

Could replace #6882 as the CircleCI integration there has gone bonkers.
@srebhan @ssoroka , can we replace the old one ? Just close it and then I will pop the draft off from this MR.

@red15
Copy link
Contributor Author

red15 commented Feb 2, 2021

Could replace #6882 as the CircleCI integration there has gone bonkers.
@srebhan @ssoroka , can we replace the old one ? Just close it and then I will pop the draft off from this MR.

Actually looks like CircleCI has suddenly realized it needs to test this also on the other MR.
So the old MR might contain more relevant information.

srebhan
srebhan reviewed Feb 3, 2021
View reviewed changes
Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

Hey @red15! Nice to see you could circumvent CircleCI problems. Could you please address moving the check of the udev path up to the construction place so we have all in one place. Otherwise we have to jump in code to see it's checked...

And please close #6882 in favor of this PR. Thanks.

@srebhan srebhan self-assigned this Feb 3, 2021
@srebhan srebhan added area/system fix pr to fix corresponding bug plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins labels Feb 3, 2021
@lgtm-com
Copy link

lgtm-com bot commented Feb 4, 2021

This pull request introduces 1 alert when merging 01423b3 into 3b8df55 - view on LGTM.com

new alerts:

  • 1 for Useless assignment to local variable

@lgtm-com
Copy link

lgtm-com bot commented Feb 4, 2021

This pull request introduces 1 alert when merging cd8902f into 3b8df55 - view on LGTM.com

new alerts:

  • 1 for Useless assignment to local variable

@lgtm-com
Copy link

lgtm-com bot commented Feb 9, 2021

This pull request introduces 2 alerts when merging d7eff22 into ba66d4f - view on LGTM.com

new alerts:

  • 2 for Useless assignment to local variable

@srebhan
Copy link
Member

srebhan commented Feb 10, 2021

Please fix the code and LGTM-bot comment! Finally if you think you are done and want me to review, please change the "Draft" state of your PR. :-)

@red15 red15 marked this pull request as ready for review February 12, 2021 18:13
srebhan
srebhan approved these changes Feb 15, 2021
View reviewed changes
Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thank you for your work @red15!

@srebhan srebhan added the ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. label Feb 15, 2021
ssoroka and others added 2 commits February 17, 2021 15:21
@ssoroka
Merge branch 'master' into non-systemd-with-unittest
99b1443
@red15
Fixing the missed rename after merging master
92bf042 
The PR !8835 renamed the DiskIO variable, this was not applied when the merge got done on this feature branch.
Thus the merge made by @ssoroka broke the test unit here ( 99b1443 ).
@helenosheaa helenosheaa merged commit e1a896c into influxdata:master Feb 26, 2021
@red15 red15 deleted the non-systemd-with-unittest branch March 15, 2021 17:10
arstercz pushed a commit to arstercz/telegraf that referenced this pull request Mar 5, 2023
@red15
Non systemd support with unittest (influxdata#8785)
c1fe33d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@srebhan srebhan srebhan approved these changes

@ivorybilled ivorybilled ivorybilled approved these changes

@telegraf-tiger telegraf-tiger[bot] telegraf-tiger[bot] approved these changes

Assignees

@srebhan srebhan

Labels
area/system fix pr to fix corresponding bug plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

enhancement - inputs/system/diskio_linux - configurable udev data path
5 participants
@red15 @srebhan @ivorybilled @helenosheaa @ssoroka