403737 - Make SVN hook non-blocking

Status: RESOLVED FIXED

(Webtools Graveyard :: Kubla, defect)

Description

[Wil Clouser [:clouserw]]

So, in my brief tests with a local repo when it couldn't contact the host SVN printed a "svn post-commit hook failed" message and went on with it's business.  Apparently this isn't the case on svn.mozilla.org.

sm-cms01 went down today and I decided to try committing while it wouldn't be able to contact the daemon.  It sat at "Transmitting file data" for several minutes before finishing the commit.  We need it to just move on if it can't contact the other server - printing a message is fine, but blocking/timing out is no good.

Comment 1

[Fred Wenzel [:wenzel]]

Ah, so the http connection we are trying to make just times out? That makes sense.

Not having it wait at all is hard to achieve though (since I assume SVN itself blocks until the script returns and we can't return before we have done our job), but I can think of two alternatives:
1) reducing the timeout on the HTTP request, but that could lead to false positives in case the server *is* available and just a little sluggish atm.
2) checking the servers' health before sending any requests, by pinging for example. This should be quicker than a TCP timeout, but of course adds additional overhead.

Comment 2

[Fred Wenzel [:wenzel]]

Attachment: Add default socket timeout to kubla post-commit hook

Okay according to this: <http://www.voidspace.org.uk/python/articles/urllib2.shtml#sockets-and-layers>, sockets do not time out at all by default. This will block the entire commit for a long time, as you described.

So I added a config option for the default timeout. I set it to 5, but if you consider it too low, feel free to raise it. In any case, this will bring the commit time down to a couple of seconds even in case of a kublad server error.

Comment 3

[Wil Clouser [:clouserw]]

Attachment: return true or false

I think your concept is good, but unless we exit with a value, svn doesn't print anything.  In my testing, if we exit with a non-zero value, it'll print whatever we echoed to standard error.  This patch just adds in a value.

Comment 4

[Fred Wenzel [:wenzel]]

Comment on attachment 288760 [details] [diff] [review]
return true or false

Looks good! Interesting that the documentation says the return value is "just ignored" while indeed it makes a difference between seeing and not seeing what went wrong.

Btw. you can probably just use "return not fail" instead of the if clause, but that's pure cosmetics.

Comment 5

[Wil Clouser [:clouserw]]

Changes are live. Thanks everyone.

Comment 6

[Wil Clouser [:clouserw]]

oh, and for the record, the timeout is 2 seconds