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

Timeout #74

Closed
wants to merge 2 commits into from
Closed

Conversation

technicalpickles
Copy link

This adds support for a timeout as described in https://github.com/dblock/slack-ruby-client/issues/72

Implementation is inspired by https://github.com/getsentry/raven-ruby/pull/54/files

@dblock
Copy link
Collaborator

dblock commented Mar 16, 2016

This only affects the web client, so should be part of Slack::Web::Config. It needs tests, README and CHANGELOG entry.

But I'd really rather not add this, but expose the connection during construction similar to how https://github.com/codegram/hyperclient does it, this way we don't have to add every option to the config. Or at least having a hash for connection options in the config so that you can throw whatever you want in there.

@technicalpickles
Copy link
Author

I was using this branch to address timeouts I was getting from Slack, but I couldn't actually set it high enough to finish without Slack resetting the connection. I'm already working with Slack to track down what the problem is, but I'm not sure if it's a problem with the timeout as there is with file processing on their end.

@dblock
Copy link
Collaborator

dblock commented Mar 25, 2016

Bump, maybe you want to try to turn this into something merge-able @technicalpickles ?

@asheynkman
Copy link

any plans to push this fix into release ? so timeout can be controlled for bigger uploads and downloads ?

@dblock
Copy link
Collaborator

dblock commented Feb 22, 2017

No "plans", someone needs to finish this PR, see my comments above.

@technicalpickles
Copy link
Author

Unfortunately it has been long enough that I don't even remember what I was using this for, and the issue I linked to is a 404 😦

I'm going to close, but will leave the branch up if anyone wants to build on it.

@asheynkman
Copy link

hi Guys ,
sorry been little busy did not get to this yet, I need to add respect and documentation and will created pull request. We have increase faraday timeout and that is resolve download initial connection metadata.

@dblock
Copy link
Collaborator

dblock commented Mar 8, 2017

Superseded with #135.

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