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

[pdh] Make the admin share configurable #5485

Merged
merged 14 commits into from
Jan 21, 2020

Conversation

hithwen
Copy link
Contributor

@hithwen hithwen commented Jan 16, 2020

Adds the ability to change the administrative share to connect to on pdh check

@codecov
Copy link

codecov bot commented Jan 16, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@88f353e). Click here to learn what that means.
The diff coverage is 66.66%.

Impacted Files Coverage Δ
spark/datadog_checks/spark/spark.py 82.44% <ø> (ø)
apache/tests/test_apache.py 90.78% <ø> (ø)
spark/datadog_checks/spark/__about__.py 100% <ø> (ø)
snmp/datadog_checks/snmp/__about__.py 100% <ø> (ø)
ibm_mq/tests/test_ibm_mq.py 97.8% <ø> (ø)
apache/datadog_checks/apache/apache.py 96% <ø> (ø)
snmp/datadog_checks/snmp/snmp.py 87.84% <ø> (ø)
kubelet/datadog_checks/kubelet/kubelet.py 83.1% <ø> (ø)
...tadog_checks_base/datadog_checks/base/__about__.py 100% <ø> (ø)
ibm_mq/datadog_checks/ibm_mq/metrics.py 92.59% <ø> (ø)
... and 7 more

@hithwen
Copy link
Contributor Author

hithwen commented Jan 17, 2020

The IBM was issue is known, there is a temporary fix in #5493

@FlorianVeaux FlorianVeaux changed the title Julia/fix pdh connection string Make the admin share configurable Jan 17, 2020
@FlorianVeaux FlorianVeaux changed the title Make the admin share configurable [pdh] Make the admin share configurable Jan 17, 2020
@FlorianVeaux
Copy link
Member

Adding [pdh] to the title to make it clear in datadog_checks_base changelog

Copy link
Contributor

@florimondmanca florimondmanca 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 subtle potential bug, then this looks good to me! (Nice prose that sheds some light on Windows obscure behaviors. 💡)

@@ -26,7 +26,7 @@ def get_requirements(fpath):
return f.readlines()


CHECKS_BASE_REQ = 'datadog_checks_base'
CHECKS_BASE_REQ = 'datadog_checks_base>10.2'
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume we are targeting 10.3 and up, but this would also match e.g. 10.2.1, so we should probably change this to…

Suggested change
CHECKS_BASE_REQ = 'datadog_checks_base>10.2'
CHECKS_BASE_REQ = 'datadog_checks_base>=10.3'

Copy link
Member

Choose a reason for hiding this comment

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

I think that'll have to be in a separate PR until datadog_checks_base is released.

Copy link
Contributor

@florimondmanca florimondmanca Jan 21, 2020

Choose a reason for hiding this comment

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

I removed the pin for now to unblock this, we can add the pin back once released.

Co-Authored-By: Florimond Manca <[email protected]>
@therve therve changed the title [pdh] Make the admin share configurable Make the admin share configurable Jan 21, 2020
@therve therve changed the title Make the admin share configurable [pdh] Make the admin share configurable Jan 21, 2020
florimondmanca
florimondmanca previously approved these changes Jan 21, 2020
Copy link
Contributor

@florimondmanca florimondmanca left a comment

Choose a reason for hiding this comment

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

:shipit:

@florimondmanca florimondmanca merged commit 1abcf9a into master Jan 21, 2020
@florimondmanca florimondmanca deleted the julia/fix-pdh-connection-string branch January 21, 2020 19:53
florimondmanca added a commit that referenced this pull request Jan 21, 2020
* Add the option to provide a different administrative share

* Add admin_share config option

* Bump check base dependency

* Add tests for new config option

* Fix typo

* call initialize_pdh_tests

* Allow for empty admin share

* Inline rstrip

* Update conf.yaml.example

* Apply suggestions from code review

Co-Authored-By: FlorianVeaux <[email protected]>

* Update conf.yaml.example

* Update pdh_check/setup.py

Co-Authored-By: Florimond Manca <[email protected]>

* Remove pin on unreleased base version

Co-authored-by: FlorianVeaux <[email protected]>
Co-authored-by: Florimond Manca <[email protected]>
AlexandreYang added a commit that referenced this pull request Jan 27, 2020
AlexandreYang pushed a commit that referenced this pull request Jan 30, 2020
* Add the option to provide a different administrative share

* Add admin_share config option

* Bump check base dependency

* Add tests for new config option

* Fix typo

* call initialize_pdh_tests

* Allow for empty admin share

* Inline rstrip

* Update conf.yaml.example

* Apply suggestions from code review

Co-Authored-By: FlorianVeaux <[email protected]>

* Update conf.yaml.example

* Update pdh_check/setup.py

Co-Authored-By: Florimond Manca <[email protected]>

* Remove pin on unreleased base version

Co-authored-by: FlorianVeaux <[email protected]>
Co-authored-by: Florimond Manca <[email protected]>
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