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

Fix timeout for sync request #132

Closed
wants to merge 1 commit into from
Closed

Fix timeout for sync request #132

wants to merge 1 commit into from

Conversation

slipeer
Copy link

@slipeer slipeer commented May 18, 2017

Fix for: #131

According Requests doc it accept timeout parameter in seconds.
But in api.py#L78 to this parameter passed huge value 30000 sec = 8.3 hours from api.py#L63
I think line api.py#L78 must be like: "timeout": timeout_ms/1000.0

@matrixbot
Copy link
Member

Can one of the admins verify this patch?

@non-Jedi
Copy link
Collaborator

non-Jedi commented Sep 19, 2017

This should address the specific problem the user had with /sync in #129, but
we should also implement a general timeout for all requests. @slipeer, could you
sign-off on this per
https://github.com/matrix-org/matrix-python-sdk/blob/master/CONTRIBUTING.rst#sign-off

@slipeer
Copy link
Author

slipeer commented Sep 21, 2017

Sorry, I've been waiting for more than three months to respond to a one-line editing of a typo that causes significant problems with timeouts. Apparently this project is sooner dead and I'm not ready to spend time on a project that no one needs.

@slipeer slipeer closed this Sep 21, 2017
@non-Jedi
Copy link
Collaborator

@slipeer sorry to hear that. The maintainer for this project has been too
busy to accepept PRs for a few months. Just today I've stepped in for him as
a co-maintainer, so this shouldn't happen in the future.

Thanks for taking the time to make the PR, and sorry you had a bad
experience.

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