issue_comments: 1357084279
This data as json
| html_url | issue_url | id | node_id | user | created_at | updated_at | author_association | body | reactions | issue | performed_via_github_app |
|---|---|---|---|---|---|---|---|---|---|---|---|
| https://github.com/simonw/datasette/issues/1955#issuecomment-1357084279 | https://api.github.com/repos/simonw/datasette/issues/1955 | 1357084279 | IC_kwDOBm6k_c5Q43Z3 | 178162 | 2022-12-19T04:34:16Z | 2022-12-19T04:34:16Z | NONE | You were super-close on the python version of the test here, changing `http` to `https` on 8b73fc6b47dffd8836f5c58aae1e57c1f66a5754 is enough to pass the test: ```diff diff --git a/tests/conftest.py b/tests/conftest.py index 69dee68b4a3f..ba07a11d37f6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -207,7 +207,7 @@ def ds_localhost_https_server(tmp_path_factory): stderr=subprocess.STDOUT, cwd=tempfile.gettempdir(), ) - wait_until_responds("http://localhost:8042/", verify=client_cert) + wait_until_responds("https://localhost:8042/", verify=client_cert) # Check it started successfully assert not ds_proc.poll(), ds_proc.stdout.read().decode("utf-8") yield ds_proc, client_cert ``` My speculation about what was happening here: when `wait_until_responds()` would time out due to SSL connection problems, because `.terminate()` isn’t in a `finally`, the datasette process wouldn’t get killed. That could (1) hang CI and (2) cause all your future local test runs to mysteriously fail because they’d be secretly talking to that old datasette process still hanging around from a past test run with an old temporary server certificate, and that old server cert wouldn’t validate against your newly-created ca cert. A `finally` for `.terminate()` would help; a fancier version could be a context manager for running the external `datasette` process that could: - ensure the process always exited when no longer needed - if you want to be fancy, call `terminate()`, `wait()` for a short timeout for the process to exit, then try `kill()` and `wait()` again; raise an exception complaining about the seemingly-unkillable process if all that fails - raise an error if the process exited with a non-zero error code; here it’s likely that some `datasette` executions were secretly failing with `[Errno 48] error while attempting to bind on address ('127.0.0.1', 8042): address already in use` | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | 1496652622 |