jaydebeapi
jaydebeapi copied to clipboard
add query timeout to execute and executemany methods
From the PreparedStatement
jdbc docs:
By default there is no limit on the amount of time allowed for a running statement to complete.
This changes allow the user to specify a query_timeout
in seconds.
Coverage increased (+0.2%) to 74.885% when pulling 064364e1a0a014a58da32f0522c13aa337286072 on fernandezpablo85:master into e99a05d5a84e9aa37ff0bac00bd5591336f54402 on baztian:master.
Coverage increased (+2.8%) to 77.471% when pulling 2b4a7e561f64cc9898194e92a6997818af26f8b0 on fernandezpablo85:master into e99a05d5a84e9aa37ff0bac00bd5591336f54402 on baztian:master.
@baztian ?
@baztian?
@baztian done. Let me know what you think.
Thanks again. I don't understand why timeout is extracted from the driver url. Shouldn't the driver already take care of the timeouts if you specify such an url. It is definitely passed to the driver? And would be wondering if this is a JDBC standard way to specify timeouts. What keeps a vendor from not naming the timeout parameter something else?
Sorry. You're right. Will send an updated patch later today.
@baztian I think that is what you originally meant, right?
One more thought: I know I asked you to change it to be a connection parameter to stay as close to DB-API spec as possible. But I also remember the discussion in #29 where the same options exist. It might be the best way to have the timeout available in both: connection and on a per query basis. This way you have the choice to be DB-API conformant and to have a timeout on a per query basis (but without being DB-API conformant). For me it would be ok to only have the connection version for now. If one needs this later on we can still add it. But feel free to add both variants.
@baztian I'm having a tough time setting up the test environment. I created a test that passes a timeout and sees that at least the statement succeeds. Hopefully the CI will run it.
I was thinking of a more thorough test modifying the .java
connection Mock to Thread.sleep
conditionally if the passed query contains the string "timeout". It was a bit cumbersome but if you want to I can add it.
Till this PR gets released, we can do this
def _set_stmt_parms(self, prep_stmt, parameters):
for i in range(len(parameters)):
prep_stmt.setObject(i + 1, parameters[i])
prep_stmt.setQueryTimeout(QUERY_TIMEOUT)
jaydebeapi.Cursor._set_stmt_parms = _set_stmt_parms
or use new
+1