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

Make max file count configurable in the directory check #6847

Merged
merged 3 commits into from
Jun 9, 2020

Conversation

nicholas-devlin
Copy link
Contributor

What does this PR do?

makes max_filegauge_count an undocumented configurable setting

Motivation

I wanted to monitor more than 20 files and expected this setting to be configureable like in our rabbitmq check and the jmx fetch based integrations

Additional Notes

I'm not sure if any unit tests or things also need to be updated, just let me know if so.

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have changelog/ and integration/ labels attached

I wanted to monitor more than 20 files and expected this setting to be configureable like in our [rabbitmq check](https://github.com/DataDog/integrations-core/blob/master/rabbitmq/datadog_checks/rabbitmq/rabbitmq.py#L183-L187) and the [jmx fetch based integrations](https://github.com/DataDog/jmxfetch/blob/master/src/main/java/org/datadog/jmxfetch/Instance.java#L64)
@nicholas-devlin nicholas-devlin requested a review from a team as a code owner June 8, 2020 16:43
Copy link
Contributor

@ofek ofek left a comment

Choose a reason for hiding this comment

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

@nicholas-devlin nicholas-devlin changed the title Update directory.py Make max file count configurable in the directory check Jun 8, 2020
forgot self when referencing the max constant in the default value

Co-authored-by: Christine Chen <[email protected]>
@nicholas-devlin
Copy link
Contributor Author

thanks for the reviews! I updated the PR title, and updated the reference to that max constant

@ChristineTChen
Copy link
Contributor

@nicholas-devlin looks good to me, I'm ready to ✅ . CI is failing because of style. Please run ddev test directory -fs and commit the changes.

@nicholas-devlin
Copy link
Contributor Author

nicholas-devlin commented Jun 8, 2020

I couldn't figure out how to run that command:

$ ddev test directory -fs
Code formatting is not enabled!
To enabled it, put `dd_check_style = true` under the `[testenv]` section of `tox.ini`.

nothing seems to happen when i run the test without the directory check specified, either. Maybe it isn't detecting that there are changes to the file compared to master?

$ ddev test
Nothing to test!

I don't really understand the error that the docs / build check is giving me: https://github.com/DataDog/integrations-core/pull/6847/checks?check_run_id=751798110

@ofek
Copy link
Contributor

ofek commented Jun 8, 2020

mkdocstrings/pytkdocs#43

@ofek
Copy link
Contributor

ofek commented Jun 8, 2020

@nicholas-devlin Show ddev --version

@ChristineTChen ChristineTChen merged commit a664d64 into master Jun 9, 2020
@ChristineTChen ChristineTChen deleted the nicholas-devlin-patch-1 branch June 9, 2020 01:47
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.

4 participants