Fix documentation for timeout parameters #535
Conversation
There was a problem hiding this comment.
I think connect_timeout is better name for MySQL client compatibility.
http://dev.mysql.com/doc/refman/5.7/en/mysql-command-options.html#option_mysql_connect_timeout
README.md
Outdated
| Default: 0 | ||
| ``` | ||
|
|
||
| I/O read timeout. The value must be a decimal number with an unit suffix ( *"ms"*, *"s"*, *"m"*, *"h"* ), such as *"30s"*, *"0.5m"* or *"1m30s"*. | ||
| I/O read timeout. The value must be a decimal number with a unit suffix ( *"ms"*, *"s"*, *"m"*, *"h"* ), such as *"30s"*, *"0.5m"* or *"1m30s"*. |
There was a problem hiding this comment.
Yes, because "unit" does not start with a vowel, phonetically: /ˈjuː.nɪt/.
|
connect_timeout may be a better name, but I really don't want to change the API here. This is just a clarification in the README. |
|
Anything else you need me to do here? |
README.md
Outdated
| Default: OS default | ||
| ``` | ||
|
|
||
| *Driver* side connection timeout. The value must be a decimal number with an unit suffix ( *"ms"*, *"s"*, *"m"*, *"h"* ), such as *"30s"*, *"0.5m"* or *"1m30s"*. To set a server side timeout, use the parameter [`wait_timeout`](http://dev.mysql.com/doc/refman/5.6/en/server-system-variables.html#sysvar_wait_timeout). | ||
| Timeout for establishing connections, aka dial timeout. The value must be a decimal number with a unit suffix ( *"ms"*, *"s"*, *"m"*, *"h"* ), such as *"30s"*, *"0.5m"* or *"1m30s"*. To configure the duration after which connections are removed from the connection pool, use [*sql.DB.SetConnMaxLifetime](https://golang.org/pkg/database/sql/#DB.SetConnMaxLifetime). |
There was a problem hiding this comment.
I think second sentence ("To configure...") is redundant.
Both of wait_timeout sysvar and SetConnMaxLifetime are totally unrelated to this config variable.
There was a problem hiding this comment.
That SetConnMaxLifetime is unrelated to the driver config is exactly what makes this link useful. It says "the driver doesn't do what you may be looking for; see here instead."
What's the harm in this link? The Internet works so well because of hyperlinks.
If you insist I'll remove it, but in my opinion it is perfectly adequate to mention SetConnMaxLifetime in the documentation for a parameter that is simply called "timeout", which by itself can mean a dozen different things.
There was a problem hiding this comment.
If there are 1000 people, there may be 1000 "may be looking for".
Adding 1000 links for each option makes README unreadable.
So my question is why your case is so special?
There was a problem hiding this comment.
If this is really FAQ, I'm OK to adding link. But I have seen only one question. (#488)
I think SetConnMaxLifetime is very useful (that's why I added it!), I feel there should be more appreciate section like "recommended configurations".
There was a problem hiding this comment.
Maybe, adding "Configuring connection pool and timeout" section under the "Usage" section is more useufl.
There was a problem hiding this comment.
Fair enough. Done in 4416483.
|
Just as a side-note: We can't merge anything without attributing the copyright to someone |
|
As in |
|
As in an entry in the AUTHORS file |
|
I've added myself to AUTHORS. |
|
Please push the change to your branch. It can be merged then. |
|
Commited to the wrong branch by accident. It's here now. |
Description
The description for the "timeout" parameter is misleading. The parameter has nothing to do idle timeouts, which is implied by linking to the documentation for "wait_timeout". It's only use is in the net.Dial call and it is documented as such in the config struct
Checklist