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

Set logging level for requests via new Server options #11

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sushengloong
Copy link

Relating to gaqzi/gocd-cli#14, this PR will allow client to set logging level for requests via the new Server options using request_debug_level key which defaults to 0.
This should be helpful for debugging.

Sad to say my Python knowledge is very limited so I don't know how to unit test this. 😓
Any suggestion?

@henriquegemignani
Copy link
Collaborator

This appears to be fine on first look, but looking deeper request_debug_level is only used by _add_basic_auth and that is only used if you pass a user and password.

As for test, I'd say the important piece is that request_debug_level reaches urllib. So I suggest you mock the part of urllib that receives the debug level and then assert that it received the correct value.

@gaqzi
Copy link
Owner

gaqzi commented Mar 7, 2016

@sushengloong this is the wrong place for this, since I know your use case, it should be handled in gocd-cli. Although adding this as an option and making it more available is a good idea!

The more pythonic way would be to have it as an argument with a default value, as the options = {} pattern can be handled completely with keyword arguments (which have been part of Python for a long time). :)

gaqzi added a commit that referenced this pull request Mar 8, 2016
Previously it wasn't being set properly as the value was being set at
init, despite there being an attribute for it on the server class.

By setting the debug flag on init and then ensuring that the urllib
opener is always being set (with or without the auth handler) the debug
output will actually be shown.

This relates to pr gaqzi/gocd/#11 and issue gaqzi/gocd-cli/#14.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants