From 244e6774f373aebd3745a1c0d89ed788f4b41334 Mon Sep 17 00:00:00 2001 From: AKoulou Date: Wed, 17 May 2023 21:25:35 -0400 Subject: [PATCH 01/13] Fixes signal that only works in main thread of the main interpreter --- parfive/downloader.py | 7 +++++++ parfive/tests/test_downloader.py | 25 +++++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/parfive/downloader.py b/parfive/downloader.py index 586f123..a3836a4 100644 --- a/parfive/downloader.py +++ b/parfive/downloader.py @@ -5,6 +5,8 @@ import pathlib import contextlib import urllib.parse +import threading +import warnings from typing import Union, Callable, Optional from functools import reduce @@ -223,6 +225,11 @@ def filepath(url, resp): def _add_shutdown_signals(loop, task): if os.name == "nt": return + + if threading.current_thread() != threading.main_thread(): + warnings.warn("Signal only works in main thread of the main interpreter and the handlers to interrupt the kernel have not been added.") + return + for sig in (signal.SIGINT, signal.SIGTERM): loop.add_signal_handler(sig, task.cancel) diff --git a/parfive/tests/test_downloader.py b/parfive/tests/test_downloader.py index 602e556..e839e39 100644 --- a/parfive/tests/test_downloader.py +++ b/parfive/tests/test_downloader.py @@ -1,5 +1,6 @@ import os import platform +import threading from pathlib import Path from unittest import mock from unittest.mock import patch @@ -463,3 +464,27 @@ 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) + +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() + thread.join() + + validate_test_file(thread.result) \ No newline at end of file From 86904cac04ff8176c755f5db9ac642d05b322a87 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sat, 20 May 2023 13:25:24 +0000 Subject: [PATCH 02/13] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- parfive/downloader.py | 12 +++++++----- parfive/tests/test_downloader.py | 3 ++- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/parfive/downloader.py b/parfive/downloader.py index a3836a4..cd2a1da 100644 --- a/parfive/downloader.py +++ b/parfive/downloader.py @@ -3,10 +3,10 @@ import asyncio import logging import pathlib +import warnings +import threading import contextlib import urllib.parse -import threading -import warnings from typing import Union, Callable, Optional from functools import reduce @@ -225,11 +225,13 @@ def filepath(url, resp): def _add_shutdown_signals(loop, task): if os.name == "nt": return - + if threading.current_thread() != threading.main_thread(): - warnings.warn("Signal only works in main thread of the main interpreter and the handlers to interrupt the kernel have not been added.") + warnings.warn( + "Signal only works in main thread of the main interpreter and the handlers to interrupt the kernel have not been added." + ) return - + for sig in (signal.SIGINT, signal.SIGTERM): loop.add_signal_handler(sig, task.cancel) diff --git a/parfive/tests/test_downloader.py b/parfive/tests/test_downloader.py index e839e39..cf48bb3 100644 --- a/parfive/tests/test_downloader.py +++ b/parfive/tests/test_downloader.py @@ -474,6 +474,7 @@ def __init__(self, *args, **kwargs): def run(self): self.result = self._target(*self._args, **self._kwargs) + def test_download_out_of_main_thread(httpserver, tmpdir): tmpdir = str(tmpdir) httpserver.serve_content( @@ -487,4 +488,4 @@ def test_download_out_of_main_thread(httpserver, tmpdir): thread.start() thread.join() - validate_test_file(thread.result) \ No newline at end of file + validate_test_file(thread.result) From 8b394b4688f5bbc8d4443c71c9a6051cafd6d121 Mon Sep 17 00:00:00 2001 From: Athanasios Kouloumvakos Date: Wed, 13 Sep 2023 21:29:03 -0400 Subject: [PATCH 03/13] Update warning message Co-authored-by: Stuart Mumford --- parfive/downloader.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parfive/downloader.py b/parfive/downloader.py index cd2a1da..f4593e4 100644 --- a/parfive/downloader.py +++ b/parfive/downloader.py @@ -228,7 +228,7 @@ def _add_shutdown_signals(loop, task): if threading.current_thread() != threading.main_thread(): warnings.warn( - "Signal only works in main thread of the main interpreter and the handlers to interrupt the kernel have not been added." + "This download has been started in a thread which is not the main thread. You will not be able to interrupt the download." ) return From 3581273989cb985f5a446b07bf0ff80cf7b7ef99 Mon Sep 17 00:00:00 2001 From: AKoulou Date: Thu, 5 Oct 2023 10:30:37 -0400 Subject: [PATCH 04/13] 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. --- parfive/downloader.py | 3 ++- parfive/tests/test_downloader.py | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/parfive/downloader.py b/parfive/downloader.py index f4593e4..e3290bb 100644 --- a/parfive/downloader.py +++ b/parfive/downloader.py @@ -228,7 +228,8 @@ def _add_shutdown_signals(loop, task): 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." + "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 diff --git a/parfive/tests/test_downloader.py b/parfive/tests/test_downloader.py index cf48bb3..5d79e82 100644 --- a/parfive/tests/test_downloader.py +++ b/parfive/tests/test_downloader.py @@ -474,7 +474,7 @@ def __init__(self, *args, **kwargs): def run(self): self.result = self._target(*self._args, **self._kwargs) - +import warnings def test_download_out_of_main_thread(httpserver, tmpdir): tmpdir = str(tmpdir) httpserver.serve_content( @@ -486,6 +486,8 @@ def test_download_out_of_main_thread(httpserver, tmpdir): thread = CustomThread(target=dl.download) thread.start() - thread.join() + + 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) From 8f38c21094eb799870bf1959cf7a7c94bca38ac1 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 5 Oct 2023 14:32:20 +0000 Subject: [PATCH 05/13] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- parfive/downloader.py | 2 +- parfive/tests/test_downloader.py | 10 ++++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/parfive/downloader.py b/parfive/downloader.py index e3290bb..29c3bba 100644 --- a/parfive/downloader.py +++ b/parfive/downloader.py @@ -229,7 +229,7 @@ def _add_shutdown_signals(loop, task): 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 + UserWarning, ) return diff --git a/parfive/tests/test_downloader.py b/parfive/tests/test_downloader.py index 5d79e82..6351d70 100644 --- a/parfive/tests/test_downloader.py +++ b/parfive/tests/test_downloader.py @@ -474,7 +474,10 @@ def __init__(self, *args, **kwargs): def run(self): self.result = self._target(*self._args, **self._kwargs) + import warnings + + def test_download_out_of_main_thread(httpserver, tmpdir): tmpdir = str(tmpdir) httpserver.serve_content( @@ -486,8 +489,11 @@ def test_download_out_of_main_thread(httpserver, tmpdir): 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."): + + 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) From 87bd14c261fe87ffcff5ef04e312e8520e2a5fa5 Mon Sep 17 00:00:00 2001 From: AKoulou Date: Thu, 5 Oct 2023 13:26:53 -0400 Subject: [PATCH 06/13] Removes an unused import from my previous commit --- parfive/tests/test_downloader.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/parfive/tests/test_downloader.py b/parfive/tests/test_downloader.py index 6351d70..6a182ba 100644 --- a/parfive/tests/test_downloader.py +++ b/parfive/tests/test_downloader.py @@ -475,9 +475,6 @@ def run(self): self.result = self._target(*self._args, **self._kwargs) -import warnings - - def test_download_out_of_main_thread(httpserver, tmpdir): tmpdir = str(tmpdir) httpserver.serve_content( From a5dd3b7ef215727b070df3bfd2b82a0743a737c4 Mon Sep 17 00:00:00 2001 From: AKoulou Date: Wed, 17 May 2023 21:25:35 -0400 Subject: [PATCH 07/13] Fixes signal that only works in main thread of the main interpreter --- parfive/downloader.py | 7 +++++++ parfive/tests/test_downloader.py | 25 +++++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/parfive/downloader.py b/parfive/downloader.py index 586f123..a3836a4 100644 --- a/parfive/downloader.py +++ b/parfive/downloader.py @@ -5,6 +5,8 @@ import pathlib import contextlib import urllib.parse +import threading +import warnings from typing import Union, Callable, Optional from functools import reduce @@ -223,6 +225,11 @@ def filepath(url, resp): def _add_shutdown_signals(loop, task): if os.name == "nt": return + + if threading.current_thread() != threading.main_thread(): + warnings.warn("Signal only works in main thread of the main interpreter and the handlers to interrupt the kernel have not been added.") + return + for sig in (signal.SIGINT, signal.SIGTERM): loop.add_signal_handler(sig, task.cancel) diff --git a/parfive/tests/test_downloader.py b/parfive/tests/test_downloader.py index 602e556..e839e39 100644 --- a/parfive/tests/test_downloader.py +++ b/parfive/tests/test_downloader.py @@ -1,5 +1,6 @@ import os import platform +import threading from pathlib import Path from unittest import mock from unittest.mock import patch @@ -463,3 +464,27 @@ 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) + +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() + thread.join() + + validate_test_file(thread.result) \ No newline at end of file From 65f4bf6a50d084db29d758819cab7257f7917582 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sat, 20 May 2023 13:25:24 +0000 Subject: [PATCH 08/13] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- parfive/downloader.py | 12 +++++++----- parfive/tests/test_downloader.py | 3 ++- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/parfive/downloader.py b/parfive/downloader.py index a3836a4..cd2a1da 100644 --- a/parfive/downloader.py +++ b/parfive/downloader.py @@ -3,10 +3,10 @@ import asyncio import logging import pathlib +import warnings +import threading import contextlib import urllib.parse -import threading -import warnings from typing import Union, Callable, Optional from functools import reduce @@ -225,11 +225,13 @@ def filepath(url, resp): def _add_shutdown_signals(loop, task): if os.name == "nt": return - + if threading.current_thread() != threading.main_thread(): - warnings.warn("Signal only works in main thread of the main interpreter and the handlers to interrupt the kernel have not been added.") + warnings.warn( + "Signal only works in main thread of the main interpreter and the handlers to interrupt the kernel have not been added." + ) return - + for sig in (signal.SIGINT, signal.SIGTERM): loop.add_signal_handler(sig, task.cancel) diff --git a/parfive/tests/test_downloader.py b/parfive/tests/test_downloader.py index e839e39..cf48bb3 100644 --- a/parfive/tests/test_downloader.py +++ b/parfive/tests/test_downloader.py @@ -474,6 +474,7 @@ def __init__(self, *args, **kwargs): def run(self): self.result = self._target(*self._args, **self._kwargs) + def test_download_out_of_main_thread(httpserver, tmpdir): tmpdir = str(tmpdir) httpserver.serve_content( @@ -487,4 +488,4 @@ def test_download_out_of_main_thread(httpserver, tmpdir): thread.start() thread.join() - validate_test_file(thread.result) \ No newline at end of file + validate_test_file(thread.result) From fb15ddb2ddb1f217b2784f112a1068540b10a2aa Mon Sep 17 00:00:00 2001 From: Athanasios Kouloumvakos Date: Wed, 13 Sep 2023 21:29:03 -0400 Subject: [PATCH 09/13] Update warning message Co-authored-by: Stuart Mumford --- parfive/downloader.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parfive/downloader.py b/parfive/downloader.py index cd2a1da..f4593e4 100644 --- a/parfive/downloader.py +++ b/parfive/downloader.py @@ -228,7 +228,7 @@ def _add_shutdown_signals(loop, task): if threading.current_thread() != threading.main_thread(): warnings.warn( - "Signal only works in main thread of the main interpreter and the handlers to interrupt the kernel have not been added." + "This download has been started in a thread which is not the main thread. You will not be able to interrupt the download." ) return From 47f269a35f2d46abe4750f83b1e8d20ddb2c2aac Mon Sep 17 00:00:00 2001 From: AKoulou Date: Thu, 5 Oct 2023 10:30:37 -0400 Subject: [PATCH 10/13] 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. --- parfive/downloader.py | 3 ++- parfive/tests/test_downloader.py | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/parfive/downloader.py b/parfive/downloader.py index f4593e4..e3290bb 100644 --- a/parfive/downloader.py +++ b/parfive/downloader.py @@ -228,7 +228,8 @@ def _add_shutdown_signals(loop, task): 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." + "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 diff --git a/parfive/tests/test_downloader.py b/parfive/tests/test_downloader.py index cf48bb3..5d79e82 100644 --- a/parfive/tests/test_downloader.py +++ b/parfive/tests/test_downloader.py @@ -474,7 +474,7 @@ def __init__(self, *args, **kwargs): def run(self): self.result = self._target(*self._args, **self._kwargs) - +import warnings def test_download_out_of_main_thread(httpserver, tmpdir): tmpdir = str(tmpdir) httpserver.serve_content( @@ -486,6 +486,8 @@ def test_download_out_of_main_thread(httpserver, tmpdir): thread = CustomThread(target=dl.download) thread.start() - thread.join() + + 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) From 016c5d9d51efb61072621bcd0a657402fd3b6aa4 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 5 Oct 2023 14:32:20 +0000 Subject: [PATCH 11/13] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- parfive/downloader.py | 2 +- parfive/tests/test_downloader.py | 10 ++++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/parfive/downloader.py b/parfive/downloader.py index e3290bb..29c3bba 100644 --- a/parfive/downloader.py +++ b/parfive/downloader.py @@ -229,7 +229,7 @@ def _add_shutdown_signals(loop, task): 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 + UserWarning, ) return diff --git a/parfive/tests/test_downloader.py b/parfive/tests/test_downloader.py index 5d79e82..6351d70 100644 --- a/parfive/tests/test_downloader.py +++ b/parfive/tests/test_downloader.py @@ -474,7 +474,10 @@ def __init__(self, *args, **kwargs): def run(self): self.result = self._target(*self._args, **self._kwargs) + import warnings + + def test_download_out_of_main_thread(httpserver, tmpdir): tmpdir = str(tmpdir) httpserver.serve_content( @@ -486,8 +489,11 @@ def test_download_out_of_main_thread(httpserver, tmpdir): 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."): + + 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) From be175a0c27766d42231a481b35b6dd76408e2fe3 Mon Sep 17 00:00:00 2001 From: AKoulou Date: Thu, 5 Oct 2023 13:26:53 -0400 Subject: [PATCH 12/13] Removes an unused import from my previous commit --- parfive/tests/test_downloader.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/parfive/tests/test_downloader.py b/parfive/tests/test_downloader.py index 6351d70..6a182ba 100644 --- a/parfive/tests/test_downloader.py +++ b/parfive/tests/test_downloader.py @@ -475,9 +475,6 @@ def run(self): self.result = self._target(*self._args, **self._kwargs) -import warnings - - def test_download_out_of_main_thread(httpserver, tmpdir): tmpdir = str(tmpdir) httpserver.serve_content( From 33cf20b4a87113ae49c368407762239997019ba6 Mon Sep 17 00:00:00 2001 From: AKoulou Date: Sun, 3 Dec 2023 08:00:42 -0500 Subject: [PATCH 13/13] 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. --- parfive/tests/test_downloader.py | 1 + 1 file changed, 1 insertion(+) diff --git a/parfive/tests/test_downloader.py b/parfive/tests/test_downloader.py index 6a182ba..58e8e6e 100644 --- a/parfive/tests/test_downloader.py +++ b/parfive/tests/test_downloader.py @@ -475,6 +475,7 @@ 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(