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

Support HTTP Basic Authentication for httpok checks #52

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

Conversation

dtran320
Copy link

@dtran320 dtran320 commented Oct 2, 2014

We are checking an endpoint that we have basic authentication on with httpok and figured this might be useful for others as well:

  • Accept a username:password string for basic authentication
  • Fix settimeout for https (was raising NoneType has no attribute settimeout)
    (Same fix as maxnaude@2bf7498)
  • Move headers creation outside of retry loop since the headers stay the same

Let me know if there are any tests that should be written for this.

Fix settimeout for https (was raising NoneType has no attribute settimeout)
See maxnaude@2bf7498

Fix args for authentication
Move headers creation outside of retry loop since the headers stay the same
@mnaberez
Copy link
Member

Thanks for the pull request. This feature looks good but we need test coverage for it. If you would be willing to write some tests, we can probably merge this.

Please remove the unrelated settimeout fix. It should be merged separately to keep the history clean.

@dtran320
Copy link
Author

Hi @mnaberez! Great to meet you. Sure, I can do that. Will update this PR soon. Thanks for your work on this project - saved us a ton of time. Hope your day's going well!

@mnaberez mnaberez added the httpok label Sep 4, 2016
@mnaberez mnaberez changed the title Support HTTP Basic Authenticiation for httpok checks Support HTTP Basic Authentication for httpok checks Sep 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants