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

9544 NPrescott adbapi logging #1074

Merged
merged 7 commits into from Oct 21, 2018

Conversation

Projects
None yet
2 participants
@NPrescott
Copy link
Contributor

NPrescott commented Oct 2, 2018

Prevent "noisy" log-level from logging username and password information on connect.

Contributor Checklist:

Nolan Prescott added some commits Oct 2, 2018

Nolan Prescott
Remove t.e.adbapi logging of connection arguments
Prevent "noisy" log-level from logging username and password
information on connect.
Nolan Prescott
Appease the linter
Prior commit results in the following linter failure, which this
commit fixes:

./src/twisted/enterprise/adbapi.py:428:29:
    E711 comparison to None should be 'if cond is not None:'
Nolan Prescott
@codecov

This comment has been minimized.

Copy link

codecov bot commented Oct 14, 2018

Codecov Report

Merging #1074 into trunk will decrease coverage by <.01%.
The diff coverage is 50%.

@@            Coverage Diff             @@
##            trunk    #1074      +/-   ##
==========================================
- Coverage   91.92%   91.91%   -0.01%     
==========================================
  Files         844      844              
  Lines      151118   151023      -95     
  Branches    13185    13182       -3     
==========================================
- Hits       138911   138815      -96     
- Misses      10111    10113       +2     
+ Partials     2096     2095       -1
@twm

twm approved these changes Oct 15, 2018

Copy link
Contributor

twm left a comment

Hi @NPrescott, this looks good to me, particularly in light of the historical context you provided in the ticket — that makes this very easy to 👍.

The only feedback I have is that the newsfragment could better indicate the why of the change: e.g., "and not the connection arguments, which may contain credentials".

I've kicked off the buildbot, which must pass before I can merge. I probably won't be around later tonight to do the actual merge, so if you have time to tweak the newsfile before the PR is merged it would be appreciated. I won't hold merging for that change, though — it is good enough as-is.

@NPrescott NPrescott closed this Oct 15, 2018

@NPrescott

This comment has been minimized.

Copy link
Contributor

NPrescott commented Oct 15, 2018

(mistakenly closed this, rather than update with the newsfragment change)

@NPrescott NPrescott reopened this Oct 15, 2018

twm added some commits Oct 20, 2018

@twm twm merged commit 5a6a9b3 into twisted:trunk Oct 21, 2018

35 checks passed

buildbot/documentation Buildbot test done.
Details
buildbot/fedora26-py2.7 Buildbot test done.
Details
buildbot/fedora26-py2.7-coverage Buildbot test done.
Details
buildbot/fedora26-py3.6 Buildbot test done.
Details
buildbot/fedora26-py3.6-coverage Buildbot test done.
Details
buildbot/fedora27-py2.7 Buildbot test done.
Details
buildbot/fedora27-py2.7-coverage Buildbot test done.
Details
buildbot/fedora27-py3.6 Buildbot test done.
Details
buildbot/fedora27-py3.6-coverage Buildbot test done.
Details
buildbot/osx10.11-py2.7-coverage Buildbot test done.
Details
buildbot/rhel7-py2.7 Buildbot test done.
Details
buildbot/rhel7-py2.7-coverage Buildbot test done.
Details
buildbot/ubuntu14.04-py2.7 Buildbot test done.
Details
buildbot/ubuntu16.04-py2.7 Buildbot test done.
Details
buildbot/ubuntu16.04-py3.5 Buildbot test done.
Details
buildbot/ubuntu16.04-py3.5-asyncio-coverage Buildbot test done.
Details
buildbot/ubuntu16.04-py3.5-coverage Buildbot test done.
Details
buildbot/ubuntu16.04-pypy3 Buildbot test done.
Details
buildbot/ubuntu18.04-py2.7 Buildbot test done.
Details
buildbot/ubuntu18.04-py2.7-coverage Buildbot test done.
Details
buildbot/ubuntu18.04-py2.7-newstyle-coverage Buildbot test done.
Details
buildbot/ubuntu18.04-py2.7-nodeps Buildbot test done.
Details
buildbot/ubuntu18.04-py2.7-nodeps-coverage Buildbot test done.
Details
buildbot/ubuntu18.04-py3.6 Buildbot test done.
Details
buildbot/ubuntu18.04-py3.6-coverage Buildbot test done.
Details
buildbot/ubuntu18.04-py3.7 Buildbot test done.
Details
buildbot/ubuntu18.04-py3.7-coverage Buildbot test done.
Details
buildbot/windows7-64-py2.7 Buildbot test done.
Details
buildbot/windows7-64-py2.7-coverage Buildbot test done.
Details
buildbot/windows7-64-py2.7-wheel Buildbot test done.
Details
ci/circleci: documentation Your tests passed on CircleCI!
Details
ci/circleci: static_checkers Your tests passed on CircleCI!
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment