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 for curl timeout parameter #36

Open
wants to merge 2 commits into
base: master
from

Conversation

@jlaso
Copy link
Contributor

commented Feb 24, 2015

In order to allow slow connections (for instance in my vagrant development) is not enough with the 5 seconds of Buzz/Curl timeout default.

@jlaso

This comment has been minimized.

Copy link
Contributor Author

commented Feb 24, 2015

I change as well the format of the response in case of exception to run the tests

$this->createConfiguration('full');
$this->assertEquals(8, $this->configuration->getParameter('knp_disqus.curl_timeout'));

This comment has been minimized.

Copy link
@weaverryan

weaverryan Feb 26, 2015

Member

8 is actually equal to Disqus::DEFAULT_TIMEOUT, so it's not the best value to test with

@@ -19,6 +19,8 @@
class Disqus
{
const DEFAULT_TIMEOUT = 8;

This comment has been minimized.

Copy link
@weaverryan

weaverryan Feb 26, 2015

Member

Should we default it to 5, since that's what it defaults to in the Buzz client?

This comment has been minimized.

Copy link
@Einenlum
'code' => $e->getCode(),
'response' => $e->getMessage(),
)
);

This comment has been minimized.

Copy link
@weaverryan

weaverryan Feb 26, 2015

Member

I think this will affect how "bad" requests are handled, as suddenly there will be a response key here: https://github.com/jlaso/KnpDisqusBundle/blob/master/Disqus.php#L118

So, I'm not sure we should do this.

@weaverryan

This comment has been minimized.

Copy link
Member

commented Apr 22, 2016

@jlaso are you still interested in this feature?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.