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

[sqlserver] add command timeout, 5sec default #1446

Merged
merged 1 commit into from
May 19, 2015
Merged

Conversation

degemer
Copy link
Member

@degemer degemer commented Mar 18, 2015

Set a timeout for each SQL command (by default 5 sec), to fix #1247 .

@@ -185,19 +186,21 @@ def get_cursor(self, instance):
try:
conn_str = self._conn_string(instance)
conn = adodbapi.connect(conn_str)
conn.CommandTimeout = int(instance.get('command_timeout',
self.DEFAULT_COMMAND_TIMEOUT))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

conn.CommandTimeout was set to 30seconds by default. As you are changing it to 5 seconds now, it may break some users configuration ? What do you think ?

@yannmh
Copy link
Member

yannmh commented May 19, 2015

Besides the concerns about potential backward incompatibilities, it looks good to me.

Also it seems like you'd need to rebase your PR to be able to merge it properly

@degemer degemer force-pushed the quentin/fix-sql-server branch 2 times, most recently from 3a0a843 to ec923d9 Compare May 19, 2015 18:41
@degemer
Copy link
Member Author

degemer commented May 19, 2015

Thanks @yannmh, updated my PR.
I bumped the default timeout to 10s (it should be more than enough).

30s default, it shouldn't break any existing check as it is the default
adodbapi timeout.
[skip ci]
(as it is a Windows check not tested)
@degemer degemer force-pushed the quentin/fix-sql-server branch from ec923d9 to 4cdb978 Compare May 19, 2015 19:33
@yannmh
Copy link
Member

yannmh commented May 19, 2015

Thanks ! 👍

yannmh added a commit that referenced this pull request May 19, 2015
[sqlserver] add command timeout, 5sec default
@yannmh yannmh merged commit c228344 into master May 19, 2015
@yannmh yannmh deleted the quentin/fix-sql-server branch May 19, 2015 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SQL Server hang
2 participants