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

Support Python 3 #2760

Merged
merged 4 commits into from
Dec 21, 2018
Merged

Support Python 3 #2760

merged 4 commits into from
Dec 21, 2018

Conversation

ofek
Copy link
Contributor

@ofek ofek commented Dec 14, 2018 •
edited
Loading

Motivation

Also added e2e

Additional Notes

Depends on #2762

@ofek ofek requested a review from a team as a code owner December 14, 2018 06:57
@ofek ofek force-pushed the ofek/openldap branch 9 times, most recently from a06388f to 41d156e Compare December 15, 2018 07:40
@codecov-io
Copy link

codecov-io commented Dec 15, 2018 •
edited
Loading

Codecov Report

Merging #2760 into master will increase coverage by 11.42%.
The diff coverage is 98.71%.

@@             Coverage Diff             @@
##           master    #2760       +/-   ##
===========================================
+ Coverage   84.79%   96.21%   +11.42%     
===========================================
  Files         657        7      -650     
  Lines       37617      423    -37194     
  Branches     4516       31     -4485     
===========================================
- Hits        31898      407    -31491     
+ Misses       4394       12     -4382     
+ Partials     1325        4     -1321

@ofek ofek force-pushed the ofek/openldap branch 13 times, most recently from dca33ed to eb1c77e Compare December 16, 2018 06:38
@ofek ofek force-pushed the ofek/openldap branch 2 times, most recently from 30ea58b to af87edc Compare December 17, 2018 02:16
@ofek ofek mentioned this pull request Dec 17, 2018
Merged
gmmeyer
gmmeyer approved these changes Dec 21, 2018
View reviewed changes
Copy link
Contributor

@gmmeyer gmmeyer left a comment

Choose a reason for hiding this comment

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

I'm approving this but from now on can you split up refactoring into a second PR? It would be much easier to review.

openldap/datadog_checks/openldap/openldap.py
TIME_METRICS_DN = "cn=time,cn=monitor"
WAITERS_METRICS_DN = "cn=waiters,cn=monitor"
SEARCH_BASE = 'cn=Monitor'
SEARCH_FILTER = '(objectClass=*)'
Copy link
Contributor

Choose a reason for hiding this comment

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

why did you move these?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Module constants are usually slower to access

@ofek ofek merged commit 1c5ec9a into master Dec 21, 2018
@ofek ofek deleted the ofek/openldap branch December 21, 2018 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gmmeyer gmmeyer gmmeyer approved these changes

Assignees
No one assigned
Labels
dev/testing integration/openldap
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ofek @codecov-io @gmmeyer @gzussa