-
Notifications
You must be signed in to change notification settings - Fork 39
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
Update for current varnish-cache and polish #55
Conversation
address recommendations from vmodtool
as of Varnish Cache 6.6.0
getting closer to vcdk this also fixes an error with parallel make.
The previous timestamp was before the timeout, so the delta should also be longer than one second.
@@ -15,8 +15,8 @@ varnish v1 -vcl+backend { | |||
curl.set_timeout(200); | |||
curl.set_connect_timeout(200); | |||
} else { | |||
curl.set_timeout(1300); | |||
curl.set_connect_timeout(1300); | |||
curl.set_timeout(1310); |
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.
I have a hard time believing that 10ms make a clean difference in general. Is there a known reason for that?
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.
The reason why I changed the timeout was that the test case reported a run time of 1.299something seconds. This seems a bit odd, but I did not consider it too relevant, have not looked at what libcurl does and just applied this change to make the test case pass.
looks good to me, but I don't have commit access anymore :-) |
I object to @gquintard's definition of a week. |
merged, thanks @nigoroll ! |
This makes the vmod work and pass the test-suite with varnish-cache as of baf60b4d2ef5f418e6ac470f3562e43c37e2ad44
Please see individual commit messages.