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

Add an example of health check. #387

Merged
merged 1 commit into from
May 9, 2018
Merged

Add an example of health check. #387

merged 1 commit into from
May 9, 2018

Conversation

aaugustin
Copy link
Member

Fix #354.

@codecov
Copy link

codecov bot commented May 5, 2018

Codecov Report

Merging #387 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #387   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          29     29           
  Lines        3066   3063    -3     
  Branches      318    318           
=====================================
- Hits         3066   3063    -3
Impacted Files Coverage Δ
websockets/test_protocol.py 100% <0%> (ø) ⬆️
websockets/protocol.py 100% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4506790...cd304ff. Read the comment docs.

@ddehghan
Copy link

ddehghan commented May 9, 2018

I would add this.

elif path not in ['/', '/socket']:
          return BAD_REQUEST, [('Content-Length', 0)], ""

This will prevent bots visiting the root of the domain from causing exceptions.

Also it would be good to name the socket endpoint with en explicit name like /socket. this way you can filter out the traffic generated by bots to the root and other random urls. These will cause exceptions and operation headaches.

@aaugustin aaugustin force-pushed the document-health-check branch from b6f4341 to cd304ff Compare May 9, 2018 19:44
@aaugustin aaugustin merged commit cd304ff into master May 9, 2018
@aaugustin aaugustin deleted the document-health-check branch May 14, 2018 20:11
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.

2 participants