Skip to content

Commit

Permalink
Fixes signal that only works in main thread of the main interpreter (#…
Browse files Browse the repository at this point in the history
…133)

* Fixes signal that only works in main thread of the main interpreter

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Update warning message

Co-authored-by: Stuart Mumford <stuart@cadair.com>

* Adds a test that a warning is raised

Adds a test that a warning is raised when download has been started in a thread which is not the main thread.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Removes an unused import from my previous commit

* Fixes signal that only works in main thread of the main interpreter

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Update warning message

Co-authored-by: Stuart Mumford <stuart@cadair.com>

* Adds a test that a warning is raised

Adds a test that a warning is raised when download has been started in a thread which is not the main thread.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Removes an unused import from my previous commit

* Skip test_download_out_of_main_thread for Windows

The test_download_out_of_main_thread fails for windows and this is normal because the UserWarning is never emitted because initially the _add_shutdown_signals is skipped for windows. So this commit fix this issue.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Stuart Mumford <stuart@cadair.com>
  • Loading branch information
3 people committed Mar 27, 2024
1 parent 3b049c5 commit e52248c
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 0 deletions.
10 changes: 10 additions & 0 deletions parfive/downloader.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import asyncio
import logging
import pathlib
import warnings
import threading
import contextlib
import urllib.parse
from typing import Union, Callable, Optional
Expand Down Expand Up @@ -223,6 +225,14 @@ def filepath(url, resp):
def _add_shutdown_signals(loop, task):
if os.name == "nt":
return

if threading.current_thread() != threading.main_thread():
warnings.warn(
"This download has been started in a thread which is not the main thread. You will not be able to interrupt the download.",
UserWarning,
)
return

for sig in (signal.SIGINT, signal.SIGTERM):
loop.add_signal_handler(sig, task.cancel)

Expand Down
32 changes: 32 additions & 0 deletions parfive/tests/test_downloader.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import os
import platform
import threading
from pathlib import Path
from unittest import mock
from unittest.mock import patch
Expand Down Expand Up @@ -463,3 +464,34 @@ def test_proxy_passed_as_kwargs_to_get(tmpdir, url, proxy):
"proxy": proxy,
},
]


class CustomThread(threading.Thread):
def __init__(self, *args, **kwargs):
self.result = None
super().__init__(*args, **kwargs)

def run(self):
self.result = self._target(*self._args, **self._kwargs)


@skip_windows
def test_download_out_of_main_thread(httpserver, tmpdir):
tmpdir = str(tmpdir)
httpserver.serve_content(
"SIMPLE = T", headers={"Content-Disposition": "attachment; filename=testfile.fits"}
)
dl = Downloader()

dl.enqueue_file(httpserver.url, path=Path(tmpdir), max_splits=None)

thread = CustomThread(target=dl.download)
thread.start()

with pytest.warns(
UserWarning,
match="This download has been started in a thread which is not the main thread. You will not be able to interrupt the download.",
):
thread.join()

validate_test_file(thread.result)

0 comments on commit e52248c

Please sign in to comment.