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

Allow shorter time limits to be set using a ?_sql_time_limit_ms =20 query string limit #95

Closed
simonw opened this issue Nov 15, 2017 · 1 comment
Labels

Comments

@simonw
Copy link
Owner

simonw commented Nov 15, 2017

This cannot be greater than the configured time limit.

@simonw simonw added the small label Nov 15, 2017
@simonw simonw added this to the v2: visualization edition milestone Nov 15, 2017
@simonw
Copy link
Owner Author

simonw commented Nov 15, 2017

This means clients can ask questions but say "don't bother if it takes longer than X" - which is really handy when you're working against unknown databases that might be small or might be enormous.

@simonw simonw changed the title Allow shorter time limits to be set using a _sql_limit_ms=20 query string limit Allow shorter time limits to be set using a ?sql_time_limit_ms =20 query string limit Nov 15, 2017
@simonw simonw changed the title Allow shorter time limits to be set using a ?sql_time_limit_ms =20 query string limit Allow shorter time limits to be set using a ?_sql_time_limit_ms =20 query string limit Nov 15, 2017
simonw added a commit that referenced this issue Nov 15, 2017
Added a unit test for the sql_time_limit_ms option.

To test this, I needed to add a custom SQLite sleep() function. I've added a
simple mechanism to the Datasette class for registering custom functions.

I also had to modify the sqlite_timelimit() function. It makes use of a magic
value, N, which is the number of SQLite virtual machine instructions that
should execute in between calls to my termination decision function.

The value of N was not finely grained enough for my test to work - so I've
added logic that says that if the time limit is less than 50ms, N is set to 1.
This got the tests working.

Refs #95
@simonw simonw closed this as completed in 9cb69cb Nov 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant