-
Notifications
You must be signed in to change notification settings - Fork 17
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 overall timeouts #47
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,9 +37,14 @@ def __init__(self,): | |
|
||
# server | ||
servers = {} | ||
servers['server_url'] = "http://130.245.145.7:8082" | ||
servers['server_url'] = "http://server.iclab.org:8082" | ||
servers['login_file'] = os.path.join(self.params['user']['centinel_home'], | ||
'login') | ||
# the entire transaction should take less than 5 min | ||
servers['total_timeout'] = 60*5 | ||
# each request should timeout if we go 15 seconds without | ||
# hearing from the other party | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "Each request should timeout in 15 seconds" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As a general thought - you should try to keep comments as concise as possible. Make the code clearer instead There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree that in general, my comments should be more concise, but I want to indicate here that this is not a timeout on the complete request: it is for the socket. requests does not offer a way to do complete request timeouts. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. but the comment doesn't say that either? it doesn't say anything about sockets There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will update the comment |
||
servers['req_timeout'] = 15 | ||
self.params['server'] = servers | ||
|
||
# proxy | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why change ordering of imports?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
made alphabetical
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍