add query timeout to execute and executemany methods

[fernandezpablo85]

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.

[coveralls]

Coverage increased (+0.2%) to 74.885% when pulling 064364e on fernandezpablo85:master into e99a05d on baztian:master.

[coveralls]

Coverage increased (+2.8%) to 77.471% when pulling 2b4a7e5 on fernandezpablo85:master into e99a05d on baztian:master.

[fernandezpablo85]

@baztian ?

[fernandezpablo85]

@baztian?

[baztian]

Thanks for the PR. And sorry for not answering so long.

[fernandezpablo85]

@baztian done. Let me know what you think.

[baztian]

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?

[fernandezpablo85]

Sorry. You're right. Will send an updated patch later today.

[fernandezpablo85]

@baztian I think that is what you originally meant, right?

[baztian]

It would be nice to have at least one test that makes sure the code works with a timeout provided. (I think asserting that timeout works is a bit too complicated) Can you add a test please?

[baztian]

I would prefer to have it fail fast: Put the conversion into the connect method and only pass down the integer so you don't need a conversion down here.

[baztian]

Does the code work without this line?

[baztian]

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.

[fernandezpablo85]

@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.

[AdeshAtole]

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

[mgarriga]

+1