From ebc3c9ae941ea223ffeb2e46cb582419254eae55 Mon Sep 17 00:00:00 2001 From: James McKinney <26463+jpmckinney@users.noreply.github.com> Date: Tue, 16 Jul 2024 03:15:45 -0400 Subject: [PATCH 01/13] ci: Test on Windows and macOS --- .github/workflows/tests.yml | 29 +++++++++-------------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index e23d7775..95840a58 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -1,31 +1,22 @@ name: Tests on: [push, pull_request] - jobs: tests: if: github.event_name == 'push' || github.event.pull_request.head.repo.full_name != github.repository - runs-on: ubuntu-latest + runs-on: ${{ matrix.os }} strategy: - fail-fast: false matrix: + os: [macos-latest, windows-latest, ubuntu-latest] python-version: [3.8, 3.9, "3.10", "3.11", "3.12"] - steps: - - uses: actions/checkout@v2 - - - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v2 + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 with: python-version: ${{ matrix.python-version }} - - - name: install requirements - run: | - python -m pip install --upgrade pip - pip install -e .[test] - - name: Run unit tests - run: | - pytest scrapyd --cov=./ --cov-report=xml - + cache: pip + cache-dependency-path: setup.py + - run: pip install -e .[test] + - name: pytest scrapyd --cov=. --cov-report=xml - name: Run integration tests run: | printf "[scrapyd]\nusername = hello12345\npassword = 67890world\n" > scrapyd.conf @@ -35,9 +26,7 @@ jobs: pytest integration_tests cat scrapyd.log kill %% - - - name: Upload coverage to Codecov - uses: codecov/codecov-action@v2 + - uses: codecov/codecov-action@v2 with: token: ${{ secrets.CODECOV_TOKEN }} directory: ./coverage/reports/ From 9e6d6a3f69588f982aed02483c64263d490a5fea Mon Sep 17 00:00:00 2001 From: James McKinney <26463+jpmckinney@users.noreply.github.com> Date: Tue, 16 Jul 2024 03:25:31 -0400 Subject: [PATCH 02/13] ci: Use coveralls since codecov errors --- .github/workflows/tests.yml | 17 ++++------------- setup.py | 1 + 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index e87a6d8f..d6b72de5 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -16,7 +16,7 @@ jobs: cache: pip cache-dependency-path: setup.py - run: pip install -e .[test] - - name: pytest scrapyd --cov=. --cov-report=xml + - run: pytest scrapyd --cov=. --cov-report=xml - name: Run integration tests run: | printf "[scrapyd]\nusername = hello12345\npassword = 67890world\n" > scrapyd.conf @@ -26,15 +26,6 @@ jobs: pytest integration_tests cat scrapyd.log kill %% - - uses: codecov/codecov-action@v4 - with: - token: ${{ secrets.CODECOV_TOKEN }} - directory: ./coverage/reports/ - env_vars: OS,PYTHON - fail_ci_if_error: false - files: ./coverage.xml - flags: unittests - name: codecov-umbrella - # https://github.com/codecov/codecov-action/issues/476 - # path_to_write_report: ./coverage/codecov_report.txt - verbose: true + - env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: coveralls --service=github diff --git a/setup.py b/setup.py index 9e135166..5d9fb43f 100644 --- a/setup.py +++ b/setup.py @@ -32,6 +32,7 @@ ], extras_require={ 'test': [ + 'coveralls', 'pytest', 'pytest-cov', 'requests', From deb922d57dbd2315e9eca9531469ba9b5f139178 Mon Sep 17 00:00:00 2001 From: James McKinney <26463+jpmckinney@users.noreply.github.com> Date: Tue, 16 Jul 2024 03:34:20 -0400 Subject: [PATCH 03/13] test: Move tests directory for easier coveralls configuration --- .github/workflows/tests.yml | 2 +- .gitignore | 2 +- docs/contributing.rst | 19 +++++++----------- setup.cfg | 4 ---- {scrapyd/tests => tests}/__init__.py | 0 {scrapyd/tests => tests}/conftest.py | 0 {scrapyd/tests => tests}/logstdout.egg | Bin {scrapyd/tests => tests}/mockserver.py | 2 +- {scrapyd/tests => tests}/mybot.egg | Bin {scrapyd/tests => tests}/mybot2.egg | Bin {scrapyd/tests => tests}/mybot3.egg | Bin {scrapyd/tests => tests}/mybotunicode.egg | Bin {scrapyd/tests => tests}/quotesbot.egg | Bin .../tests => tests}/quotesbot_asyncio.egg | Bin {scrapyd/tests => tests}/start_mock_app.py | 0 .../test_dont_load_settings.py | 0 {scrapyd/tests => tests}/test_eggstorage.py | 2 +- {scrapyd/tests => tests}/test_endpoints.py | 2 +- {scrapyd/tests => tests}/test_environ.py | 0 {scrapyd/tests => tests}/test_jobstorage.py | 0 {scrapyd/tests => tests}/test_poller.py | 0 {scrapyd/tests => tests}/test_scheduler.py | 0 {scrapyd/tests => tests}/test_scripts.py | 2 +- {scrapyd/tests => tests}/test_spiderqueue.py | 0 {scrapyd/tests => tests}/test_sqlite.py | 0 {scrapyd/tests => tests}/test_utils.py | 2 +- {scrapyd/tests => tests}/test_webservice.py | 0 {scrapyd/tests => tests}/test_website.py | 0 28 files changed, 14 insertions(+), 23 deletions(-) rename {scrapyd/tests => tests}/__init__.py (100%) rename {scrapyd/tests => tests}/conftest.py (100%) rename {scrapyd/tests => tests}/logstdout.egg (100%) rename {scrapyd/tests => tests}/mockserver.py (97%) rename {scrapyd/tests => tests}/mybot.egg (100%) rename {scrapyd/tests => tests}/mybot2.egg (100%) rename {scrapyd/tests => tests}/mybot3.egg (100%) rename {scrapyd/tests => tests}/mybotunicode.egg (100%) rename {scrapyd/tests => tests}/quotesbot.egg (100%) rename {scrapyd/tests => tests}/quotesbot_asyncio.egg (100%) rename {scrapyd/tests => tests}/start_mock_app.py (100%) rename {scrapyd/tests => tests}/test_dont_load_settings.py (100%) rename {scrapyd/tests => tests}/test_eggstorage.py (97%) rename {scrapyd/tests => tests}/test_endpoints.py (98%) rename {scrapyd/tests => tests}/test_environ.py (100%) rename {scrapyd/tests => tests}/test_jobstorage.py (100%) rename {scrapyd/tests => tests}/test_poller.py (100%) rename {scrapyd/tests => tests}/test_scheduler.py (100%) rename {scrapyd/tests => tests}/test_scripts.py (89%) rename {scrapyd/tests => tests}/test_spiderqueue.py (100%) rename {scrapyd/tests => tests}/test_sqlite.py (100%) rename {scrapyd/tests => tests}/test_utils.py (98%) rename {scrapyd/tests => tests}/test_webservice.py (100%) rename {scrapyd/tests => tests}/test_website.py (100%) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index d6b72de5..ddda0810 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -16,7 +16,7 @@ jobs: cache: pip cache-dependency-path: setup.py - run: pip install -e .[test] - - run: pytest scrapyd --cov=. --cov-report=xml + - run: pytest tests --cov scrapyd - name: Run integration tests run: | printf "[scrapyd]\nusername = hello12345\npassword = 67890world\n" > scrapyd.conf diff --git a/.gitignore b/.gitignore index 7bd1d1ab..a54ee8d1 100644 --- a/.gitignore +++ b/.gitignore @@ -16,7 +16,7 @@ venv /_trial_temp /dbs /htmlcov -/scrapyd.tests.test_* +/tests.test_* /scrapyd.conf /twistd.pid /eggs diff --git a/docs/contributing.rst b/docs/contributing.rst index 2fe3427d..12ef2192 100644 --- a/docs/contributing.rst +++ b/docs/contributing.rst @@ -5,30 +5,30 @@ Contributing .. important:: - Read through the `Scrapy Contribution Docs`_ for tips relating to writing patches, reporting bugs, and project coding style + Read through the `Scrapy Contribution Docs `__ for tips relating to writing patches, reporting bugs, and project coding style These docs describe how to setup and contribute to Scrapyd. Reporting issues & bugs ----------------------- -Issues should be reported to the Scrapyd project `issue tracker`_ on GitHub +Issues should be reported to the Scrapyd project `issue tracker `__ on GitHub. Tests ----- -Tests are implemented using the `Twisted unit-testing framework`_. Scrapyd uses ``trial`` as the test running application. +Tests are implemented using the `Twisted unit-testing framework `__. Scrapyd uses ``trial`` as the test running application. Running tests ------------- To run all tests go to the root directory of the Scrapyd source code and run: - ``trial scrapyd`` + ``trial tests`` To run a specific test (say ``tests/test_poller.py``) use: - ``trial scrapyd.tests.test_poller`` + ``trial tests.test_poller`` Writing tests @@ -38,7 +38,7 @@ All functionality (including new features and bug fixes) should include a test case to check that it works as expected, so please include tests for your patches if you want them to get accepted sooner. -Scrapyd uses unit-tests, which are located in the `scrapyd/tests`_ directory. +Scrapyd uses unit-tests, which are located in the `tests `__ directory. Their module name typically resembles the full path of the module they're testing. For example, the scheduler code is in:: @@ -46,7 +46,7 @@ testing. For example, the scheduler code is in:: And their unit-tests are in:: - scrapyd/tests/test_scheduler.py + tests/test_scheduler.py Installing locally ------------------ @@ -54,8 +54,3 @@ Installing locally To install a locally edited version of Scrapyd onto the system to use and test, inside the project root run: ``pip install -e .`` - -.. _Twisted unit-testing framework: http://twistedmatrix.com/documents/current/core/development/policy/test-standard.html -.. _scrapyd/tests: https://github.com/scrapy/scrapyd/tree/master/scrapyd/tests -.. _issue tracker: https://github.com/scrapy/scrapyd/issues -.. _Scrapy Contribution Docs: http://scrapy.readthedocs.org/en/latest/contributing.html diff --git a/setup.cfg b/setup.cfg index 2281f70a..61cf6086 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,10 +1,6 @@ [bdist_wheel] universal = 1 -[coverage:run] -include = scrapyd/* -omit = scrapyd/tests* - [flake8] max-line-length = 119 diff --git a/scrapyd/tests/__init__.py b/tests/__init__.py similarity index 100% rename from scrapyd/tests/__init__.py rename to tests/__init__.py diff --git a/scrapyd/tests/conftest.py b/tests/conftest.py similarity index 100% rename from scrapyd/tests/conftest.py rename to tests/conftest.py diff --git a/scrapyd/tests/logstdout.egg b/tests/logstdout.egg similarity index 100% rename from scrapyd/tests/logstdout.egg rename to tests/logstdout.egg diff --git a/scrapyd/tests/mockserver.py b/tests/mockserver.py similarity index 97% rename from scrapyd/tests/mockserver.py rename to tests/mockserver.py index 49e2f826..120b9870 100644 --- a/scrapyd/tests/mockserver.py +++ b/tests/mockserver.py @@ -25,7 +25,7 @@ def __enter__(self, authentication=None): """ command = [ sys.executable, '-m', - "scrapyd.tests.start_mock_app", + "tests.start_mock_app", get_ephemeral_port() ] if self.authentication is not None: diff --git a/scrapyd/tests/mybot.egg b/tests/mybot.egg similarity index 100% rename from scrapyd/tests/mybot.egg rename to tests/mybot.egg diff --git a/scrapyd/tests/mybot2.egg b/tests/mybot2.egg similarity index 100% rename from scrapyd/tests/mybot2.egg rename to tests/mybot2.egg diff --git a/scrapyd/tests/mybot3.egg b/tests/mybot3.egg similarity index 100% rename from scrapyd/tests/mybot3.egg rename to tests/mybot3.egg diff --git a/scrapyd/tests/mybotunicode.egg b/tests/mybotunicode.egg similarity index 100% rename from scrapyd/tests/mybotunicode.egg rename to tests/mybotunicode.egg diff --git a/scrapyd/tests/quotesbot.egg b/tests/quotesbot.egg similarity index 100% rename from scrapyd/tests/quotesbot.egg rename to tests/quotesbot.egg diff --git a/scrapyd/tests/quotesbot_asyncio.egg b/tests/quotesbot_asyncio.egg similarity index 100% rename from scrapyd/tests/quotesbot_asyncio.egg rename to tests/quotesbot_asyncio.egg diff --git a/scrapyd/tests/start_mock_app.py b/tests/start_mock_app.py similarity index 100% rename from scrapyd/tests/start_mock_app.py rename to tests/start_mock_app.py diff --git a/scrapyd/tests/test_dont_load_settings.py b/tests/test_dont_load_settings.py similarity index 100% rename from scrapyd/tests/test_dont_load_settings.py rename to tests/test_dont_load_settings.py diff --git a/scrapyd/tests/test_eggstorage.py b/tests/test_eggstorage.py similarity index 97% rename from scrapyd/tests/test_eggstorage.py rename to tests/test_eggstorage.py index 4b8ba3b8..6b755699 100644 --- a/scrapyd/tests/test_eggstorage.py +++ b/tests/test_eggstorage.py @@ -35,7 +35,7 @@ def delete(self, project, version=None): class TestConfigureEggStorage(unittest.TestCase): def test_egg_config_application(self): config = Config() - eggstore = 'scrapyd.tests.test_eggstorage.SomeFakeEggStorage' + eggstore = 'tests.test_eggstorage.SomeFakeEggStorage' config.cp.set('scrapyd', 'eggstorage', eggstore) app = application(config) app_eggstorage = app.getComponent(IEggStorage) diff --git a/scrapyd/tests/test_endpoints.py b/tests/test_endpoints.py similarity index 98% rename from scrapyd/tests/test_endpoints.py rename to tests/test_endpoints.py index f0564501..3de31cf6 100644 --- a/scrapyd/tests/test_endpoints.py +++ b/tests/test_endpoints.py @@ -5,7 +5,7 @@ import requests from requests.models import Response -from scrapyd.tests.mockserver import MockScrapyDServer +from tests.mockserver import MockScrapyDServer @pytest.fixture diff --git a/scrapyd/tests/test_environ.py b/tests/test_environ.py similarity index 100% rename from scrapyd/tests/test_environ.py rename to tests/test_environ.py diff --git a/scrapyd/tests/test_jobstorage.py b/tests/test_jobstorage.py similarity index 100% rename from scrapyd/tests/test_jobstorage.py rename to tests/test_jobstorage.py diff --git a/scrapyd/tests/test_poller.py b/tests/test_poller.py similarity index 100% rename from scrapyd/tests/test_poller.py rename to tests/test_poller.py diff --git a/scrapyd/tests/test_scheduler.py b/tests/test_scheduler.py similarity index 100% rename from scrapyd/tests/test_scheduler.py rename to tests/test_scheduler.py diff --git a/scrapyd/tests/test_scripts.py b/tests/test_scripts.py similarity index 89% rename from scrapyd/tests/test_scripts.py rename to tests/test_scripts.py index 347bb073..3d717ab4 100644 --- a/scrapyd/tests/test_scripts.py +++ b/tests/test_scripts.py @@ -5,7 +5,7 @@ from scrapyd.__main__ import main -__version__ = pkgutil.get_data(__package__, '../VERSION').decode('ascii').strip() +__version__ = pkgutil.get_data(__package__, '../scrapyd/VERSION').decode('ascii').strip() def test_print_version(capsys, monkeypatch): diff --git a/scrapyd/tests/test_spiderqueue.py b/tests/test_spiderqueue.py similarity index 100% rename from scrapyd/tests/test_spiderqueue.py rename to tests/test_spiderqueue.py diff --git a/scrapyd/tests/test_sqlite.py b/tests/test_sqlite.py similarity index 100% rename from scrapyd/tests/test_sqlite.py rename to tests/test_sqlite.py diff --git a/scrapyd/tests/test_utils.py b/tests/test_utils.py similarity index 98% rename from scrapyd/tests/test_utils.py rename to tests/test_utils.py index 41a80267..69767c6f 100644 --- a/scrapyd/tests/test_utils.py +++ b/tests/test_utils.py @@ -61,7 +61,7 @@ def setUp(self): def add_test_version(self, file, project, version): eggstorage = self.app.getComponent(IEggStorage) - eggfile = BytesIO(get_data("scrapyd.tests", file)) + eggfile = BytesIO(get_data("tests", file)) eggstorage.put(eggfile, project, version) def test_get_spider_list_log_stdout(self): diff --git a/scrapyd/tests/test_webservice.py b/tests/test_webservice.py similarity index 100% rename from scrapyd/tests/test_webservice.py rename to tests/test_webservice.py diff --git a/scrapyd/tests/test_website.py b/tests/test_website.py similarity index 100% rename from scrapyd/tests/test_website.py rename to tests/test_website.py From 8d1af40533cdc52d261cf2a1c36c575796d40104 Mon Sep 17 00:00:00 2001 From: James McKinney <26463+jpmckinney@users.noreply.github.com> Date: Tue, 16 Jul 2024 03:46:40 -0400 Subject: [PATCH 04/13] fix: Fix dbs_dir with drive letter on Windows --- docs/news.rst | 1 + scrapyd/utils.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/news.rst b/docs/news.rst index 044611a1..18b091c1 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -50,6 +50,7 @@ Fixed - The :ref:`schedule.json` webservice sets the ``node_name`` field in error responses. - The :ref:`cancel.json` webservice now works on Windows. +- The :ref:`dbs_dir` setting no longer causes an error if it contains a drive letter on Windows. - The :ref:`jobs_to_keep` setting no longer causes an error if a file to delete can't be deleted (for example, if the file is open on Windows). - When managing multiple projects, the next pending job for all but one project was unreported by the :ref:`daemonstatus.json` and :ref:`listjobs.json` webservices, and was not cancellable by the :ref:`cancel.json` webservice. diff --git a/scrapyd/utils.py b/scrapyd/utils.py index 2002a7bf..f8ca867b 100644 --- a/scrapyd/utils.py +++ b/scrapyd/utils.py @@ -62,7 +62,7 @@ def get_spider_queues(config): def sqlite_connection_string(config, database): dbs_dir = config.get('dbs_dir', 'dbs') - if dbs_dir == ':memory:' or urlsplit(dbs_dir).scheme: + if dbs_dir == ':memory:' or (urlsplit(dbs_dir).scheme and not os.path.splitdrive(dbs_dir)[0]): return dbs_dir if not os.path.exists(dbs_dir): os.makedirs(dbs_dir) From 9155c8f302a0febccb9b9a77c737a7c549eb9a05 Mon Sep 17 00:00:00 2001 From: James McKinney <26463+jpmckinney@users.noreply.github.com> Date: Tue, 16 Jul 2024 03:49:03 -0400 Subject: [PATCH 05/13] build: Update MANIFEST.in --- MANIFEST.in | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/MANIFEST.in b/MANIFEST.in index 1d442769..5c464cbf 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -6,7 +6,8 @@ recursive-include docs *.rst recursive-include docs *.txt recursive-include docs Makefile recursive-include scrapyd *.py -recursive-include scrapyd *.egg +recursive-include tests *.egg +recursive-include tests *.py recursive-include integration_tests *.py exclude .pre-commit-config.yaml exclude .readthedocs.yaml From 1b0e3eb85f87fb1cff068ebe7be2365f3e24f432 Mon Sep 17 00:00:00 2001 From: James McKinney <26463+jpmckinney@users.noreply.github.com> Date: Tue, 16 Jul 2024 03:57:57 -0400 Subject: [PATCH 06/13] chore: Change import style for clarity --- scrapyd/eggstorage.py | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/scrapyd/eggstorage.py b/scrapyd/eggstorage.py index 5efb67b4..575d73e4 100644 --- a/scrapyd/eggstorage.py +++ b/scrapyd/eggstorage.py @@ -1,7 +1,7 @@ +import os import re +import shutil from glob import glob -from os import listdir, makedirs, path, remove -from shutil import copyfileobj, rmtree from zope.interface import implementer @@ -17,11 +17,11 @@ def __init__(self, config): def put(self, eggfile, project, version): eggpath = self._eggpath(project, version) - eggdir = path.dirname(eggpath) - if not path.exists(eggdir): - makedirs(eggdir) + eggdir = os.path.dirname(eggpath) + if not os.path.exists(eggdir): + os.makedirs(eggdir) with open(eggpath, 'wb') as f: - copyfileobj(eggfile, f) + shutil.copyfileobj(eggfile, f) def get(self, project, version=None): if version is None: @@ -32,27 +32,25 @@ def get(self, project, version=None): return version, open(self._eggpath(project, version), 'rb') def list(self, project): - eggdir = path.join(self.basedir, project) - versions = [path.splitext(path.basename(x))[0] - for x in glob("%s/*.egg" % eggdir)] + versions = [ + os.path.splitext(os.path.basename(path))[0] + for path in glob(os.path.join(self.basedir, project, "*.egg")) + ] return sorted_versions(versions) def list_projects(self): - projects = [] - if path.exists(self.basedir): - projects.extend(d for d in listdir(self.basedir) - if path.isdir('%s/%s' % (self.basedir, d))) - return projects + if os.path.exists(self.basedir): + return [name for name in os.listdir(self.basedir) if os.path.isdir(os.path.join(self.basedir, name))] + return [] def delete(self, project, version=None): if version is None: - rmtree(path.join(self.basedir, project)) + shutil.rmtree(os.path.join(self.basedir, project)) else: - remove(self._eggpath(project, version)) + os.remove(self._eggpath(project, version)) if not self.list(project): # remove project if no versions left self.delete(project) def _eggpath(self, project, version): sanitized_version = re.sub(r'[^a-zA-Z0-9_-]', '_', version) - x = path.join(self.basedir, project, "%s.egg" % sanitized_version) - return x + return os.path.join(self.basedir, project, f"{sanitized_version}.egg") From fde9e3a1811a912bc7d5a5716ccfd418d2ecc961 Mon Sep 17 00:00:00 2001 From: James McKinney <26463+jpmckinney@users.noreply.github.com> Date: Tue, 16 Jul 2024 04:25:57 -0400 Subject: [PATCH 07/13] fix(windows): Add try/finally --- scrapyd/runner.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/scrapyd/runner.py b/scrapyd/runner.py index 38db9c75..7c852480 100644 --- a/scrapyd/runner.py +++ b/scrapyd/runner.py @@ -20,8 +20,10 @@ def project_environment(project): eggversion = os.environ.get('SCRAPYD_EGG_VERSION', None) version, eggfile = eggstorage.get(project, eggversion) if eggfile: - activate_egg(eggfile.name) - eggfile.close() + try: + activate_egg(eggfile.name) + finally: + eggfile.close() assert 'scrapy.conf' not in sys.modules, "Scrapy settings already loaded" yield From 721caaf49beb454f9dabd6ad983ceaebdf2656ad Mon Sep 17 00:00:00 2001 From: James McKinney <26463+jpmckinney@users.noreply.github.com> Date: Tue, 16 Jul 2024 04:37:48 -0400 Subject: [PATCH 08/13] fix: Egg storage doesn't need to return open files --- scrapyd/eggstorage.py | 2 +- scrapyd/interfaces.py | 2 +- scrapyd/runner.py | 9 +++------ tests/test_eggstorage.py | 18 +++++++++--------- 4 files changed, 14 insertions(+), 17 deletions(-) diff --git a/scrapyd/eggstorage.py b/scrapyd/eggstorage.py index 575d73e4..d020909a 100644 --- a/scrapyd/eggstorage.py +++ b/scrapyd/eggstorage.py @@ -29,7 +29,7 @@ def get(self, project, version=None): version = self.list(project)[-1] except IndexError: return None, None - return version, open(self._eggpath(project, version), 'rb') + return version, self._eggpath(project, version) def list(self, project): versions = [ diff --git a/scrapyd/interfaces.py b/scrapyd/interfaces.py index 18fcccc1..06a70d98 100644 --- a/scrapyd/interfaces.py +++ b/scrapyd/interfaces.py @@ -9,7 +9,7 @@ def put(eggfile, project, version): version""" def get(project, version=None): - """Return a tuple (version, file) with the the egg for the specified + """Return a tuple (version, filename) for the egg matching the specified project and version. If version is None, the latest version is returned. If no egg is found for the given project/version (None, None) should be returned.""" diff --git a/scrapyd/runner.py b/scrapyd/runner.py index 7c852480..3a08976a 100644 --- a/scrapyd/runner.py +++ b/scrapyd/runner.py @@ -18,12 +18,9 @@ def project_environment(project): eggstorage = eggstorage_cls(config) eggversion = os.environ.get('SCRAPYD_EGG_VERSION', None) - version, eggfile = eggstorage.get(project, eggversion) - if eggfile: - try: - activate_egg(eggfile.name) - finally: - eggfile.close() + version, eggpath = eggstorage.get(project, eggversion) + if eggpath: + activate_egg(eggpath) assert 'scrapy.conf' not in sys.modules, "Scrapy settings already loaded" yield diff --git a/tests/test_eggstorage.py b/tests/test_eggstorage.py index 6b755699..8d3e81d6 100644 --- a/tests/test_eggstorage.py +++ b/tests/test_eggstorage.py @@ -78,20 +78,20 @@ def test_put_get_list_delete(self): ]) self.assertEqual(self.eggst.list('mybot2'), []) - v, f = self.eggst.get('mybot') + v, name = self.eggst.get('mybot') self.assertEqual(v, "03_ver") - self.assertEqual(f.read(), b"egg03") - f.close() + with open(name, 'rb') as f: + self.assertEqual(f.read(), b"egg03") - v, f = self.eggst.get('mybot', '02_my branch') + v, name = self.eggst.get('mybot', '02_my branch') self.assertEqual(v, "02_my branch") - self.assertEqual(f.read(), b"egg02") - f.close() + with open(name, 'rb') as f: + self.assertEqual(f.read(), b"egg02") - v, f = self.eggst.get('mybot', '02_my_branch') + v, name = self.eggst.get('mybot', '02_my_branch') self.assertEqual(v, "02_my_branch") - self.assertEqual(f.read(), b"egg02") - f.close() + with open(name, 'rb') as f: + self.assertEqual(f.read(), b"egg02") self.eggst.delete('mybot', '02_my branch') self.assertEqual(self.eggst.list('mybot'), ['01', '03_ver']) From 833e354f8618efeb5c52ec8005b8a3b068b291b8 Mon Sep 17 00:00:00 2001 From: James McKinney <26463+jpmckinney@users.noreply.github.com> Date: Tue, 16 Jul 2024 15:45:04 -0400 Subject: [PATCH 09/13] test: Update test, since Windows seems to have lower datetime precision --- docs/news.rst | 1 + tests/test_sqlite.py | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/news.rst b/docs/news.rst index 18b091c1..07c4cdee 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -24,6 +24,7 @@ Documentation Changed ~~~~~~~ +- The ``IEggStorage.get()`` interface returns a ``(version, filename)`` tuple, instead of a ``(version, file)`` tuple. - Add a confirmation dialog to the Cancel button. - Add "Last modified" column to the directory listings of log files and item feeds. - Drop support for end-of-life Python version 3.7. diff --git a/tests/test_sqlite.py b/tests/test_sqlite.py index c04f6ddb..232f4987 100644 --- a/tests/test_sqlite.py +++ b/tests/test_sqlite.py @@ -1,3 +1,4 @@ +import datetime import unittest from scrapyd.jobstorage import Job @@ -154,7 +155,9 @@ class SqliteFinishedJobsTest(unittest.TestCase): def setUp(self): self.q = SqliteFinishedJobs(':memory:') - self.j1, self.j2, self.j3 = Job('p1', 's1'), Job('p2', 's2'), Job('p3', 's3') + self.j1 = Job('p1', 's1', end_time=datetime.datetime(2001, 2, 3, 4, 5, 6, 7)) + self.j2 = Job('p2', 's2', end_time=datetime.datetime(2001, 2, 3, 4, 5, 6, 8)) + self.j3 = Job('p3', 's3', end_time=datetime.datetime(2001, 2, 3, 4, 5, 6, 9)) self.q.add(self.j1) self.q.add(self.j2) self.q.add(self.j3) From ffc837a8c4fbe8cd639301b3ce36fe892a2792c5 Mon Sep 17 00:00:00 2001 From: James McKinney <26463+jpmckinney@users.noreply.github.com> Date: Tue, 16 Jul 2024 16:26:29 -0400 Subject: [PATCH 10/13] docs: Document use of sys.platform --- scrapyd/webservice.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scrapyd/webservice.py b/scrapyd/webservice.py index 39355288..620c2d45 100644 --- a/scrapyd/webservice.py +++ b/scrapyd/webservice.py @@ -104,6 +104,9 @@ def render_POST(self, txrequest): args = {k: v[0] for k, v in native_stringify_dict(copy(txrequest.args), keys_only=False).items()} project = _get_required_param(args, 'project') jobid = _get_required_param(args, 'job') + # Instead of os.name, use sys.platform, which disambiguates Cygwin, which implements SIGINT not SIGBREAK. + # https://cygwin.com/cygwin-ug-net/kill.html + # https://github.com/scrapy/scrapy/blob/06f9c289d1c92dbb8e41a837b886e5cadb81a061/tests/test_crawler.py#L886 signal = args.get('signal', 'INT' if sys.platform != 'win32' else 'BREAK') prevstate = None try: From 4c61ee47486bc2ea4951fa28074e2dad91495cde Mon Sep 17 00:00:00 2001 From: James McKinney <26463+jpmckinney@users.noreply.github.com> Date: Tue, 16 Jul 2024 16:35:13 -0400 Subject: [PATCH 11/13] test: Update expectations for Windows --- docs/news.rst | 24 ++++++++++++++---------- integration_tests/test_webservice.py | 15 +++++++++++++-- 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/docs/news.rst b/docs/news.rst index 07c4cdee..d35ff354 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -24,27 +24,31 @@ Documentation Changed ~~~~~~~ -- The ``IEggStorage.get()`` interface returns a ``(version, filename)`` tuple, instead of a ``(version, file)`` tuple. +- **BACKWARDS-INCOMPATIBLE CHANGE:** The ``IEggStorage.get()`` interface returns a ``(version, filename)`` tuple, instead of a ``(version, file)`` tuple. +- Drop support for end-of-life Python version 3.7. + +Web UI +^^^^^^ + - Add a confirmation dialog to the Cancel button. - Add "Last modified" column to the directory listings of log files and item feeds. -- Drop support for end-of-life Python version 3.7. API ^^^ - If the ``egg`` parameter to the :ref:`addversion.json`` webservice is not a ZIP file, use the error message, "egg is not a ZIP file (if using curl, use egg=@path not egg=path)". -- Clarify some error messages, for example: +- Clarify error messages, for example: - - ``'project' parameter is required`` instead of ``'project'`` - - ``project 'myproject' not found`` instead of ``'myproject'`` - - ``exception class: message`` instead of ``message`` - - ``ValueError: Unknown or corrupt egg`` instead of ``TypeError: 'tuple' object is not an iterator`` + - ``'project' parameter is required``, instead of ``'project'`` + - ``project 'myproject' not found``, instead of ``'myproject'`` + - ``exception class: message``, instead of ``message`` + - ``ValueError: Unknown or corrupt egg``, instead of ``TypeError: 'tuple' object is not an iterator`` CLI ^^^ -- Correct the usage message and long description, remove all ``twistd`` subcommands, and remove the ``--nodaemon`` and ``--python=`` options, which it overrides. -- Run the ``scrapyd.__main__`` module instead of the ``scrapyd.scripts.scrapyd_run`` module. +- Correct the usage message and long description, remove all ``twistd`` subcommands, and remove the ``--nodaemon`` and ``--python=`` options, which are overridden. +- Run the ``scrapyd.__main__`` module, instead of the ``scrapyd.scripts.scrapyd_run`` module. Fixed ~~~~~ @@ -53,7 +57,7 @@ Fixed - The :ref:`cancel.json` webservice now works on Windows. - The :ref:`dbs_dir` setting no longer causes an error if it contains a drive letter on Windows. - The :ref:`jobs_to_keep` setting no longer causes an error if a file to delete can't be deleted (for example, if the file is open on Windows). -- When managing multiple projects, the next pending job for all but one project was unreported by the :ref:`daemonstatus.json` and :ref:`listjobs.json` webservices, and was not cancellable by the :ref:`cancel.json` webservice. +- The next pending job for all but one project was unreported by the :ref:`daemonstatus.json` and :ref:`listjobs.json` webservices, and was not cancellable by the :ref:`cancel.json` webservice. 1.4.3 (2023-09-25) ------------------ diff --git a/integration_tests/test_webservice.py b/integration_tests/test_webservice.py index e95a23c5..c5b87731 100644 --- a/integration_tests/test_webservice.py +++ b/integration_tests/test_webservice.py @@ -1,3 +1,5 @@ +import sys + import pytest import requests import scrapy @@ -9,6 +11,9 @@ def assert_webservice(method, path, expected, **kwargs): response = req(method, path, **kwargs) json = response.json() json.pop("node_name") + if "message" in expected: + if sys.platform == 'win32': + expected["message"] = expected["message"].replace("\n", "\r\n") assert json == expected @@ -125,7 +130,10 @@ def test_delversion_nonexistent(): "/delversion.json", { "status": "error", - "message": "FileNotFoundError: [Errno 2] No such file or directory: 'eggs/nonexistent/noegg.egg'", + "message": "FileNotFoundError: " + ( + "[Errno 2] No such file or directory: 'eggs/nonexistent/noegg.egg'" if sys.platform != 'win32' + else "[WinError 3] The system cannot find the path specified: 'eggs\\\\nonexistent\\\\noegg.egg'" + ), }, data={"project": "nonexistent", "version": "noegg"}, ) @@ -137,7 +145,10 @@ def test_delproject_nonexistent(): "/delproject.json", { "status": "error", - "message": "FileNotFoundError: [Errno 2] No such file or directory: 'eggs/nonexistent'", + "message": "FileNotFoundError: " + ( + "[Errno 2] No such file or directory: 'eggs/nonexistent'" if sys.platform != 'win32' + else "[WinError 3] The system cannot find the path specified: 'eggs\\\\nonexistent'" + ), }, data={"project": "nonexistent"}, ) From bb579ad5ab0e49e53f97a902734cb538307bba45 Mon Sep 17 00:00:00 2001 From: James McKinney <26463+jpmckinney@users.noreply.github.com> Date: Tue, 16 Jul 2024 16:44:24 -0400 Subject: [PATCH 12/13] fix(7cbaee7): Don't encode 204 NO_CONTENT --- scrapyd/utils.py | 5 ++++- scrapyd/webservice.py | 1 - 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/scrapyd/utils.py b/scrapyd/utils.py index f8ca867b..b406844f 100644 --- a/scrapyd/utils.py +++ b/scrapyd/utils.py @@ -22,7 +22,10 @@ def render(self, txrequest): return self.render_object(r, txrequest) def render_object(self, obj, txrequest): - r = self.json_encoder.encode(obj) + "\n" + if obj is None: + r = '' + else: + r = self.json_encoder.encode(obj) + "\n" txrequest.setHeader('Content-Type', 'application/json') txrequest.setHeader('Access-Control-Allow-Origin', '*') txrequest.setHeader('Access-Control-Allow-Methods', 'GET, POST, PATCH, PUT, DELETE') diff --git a/scrapyd/webservice.py b/scrapyd/webservice.py index 620c2d45..eb045f54 100644 --- a/scrapyd/webservice.py +++ b/scrapyd/webservice.py @@ -55,7 +55,6 @@ def render_OPTIONS(self, txrequest): methods.append('POST') txrequest.setHeader('Allow', ', '.join(methods)) txrequest.setResponseCode(http.NO_CONTENT) - return b'' def _error(self, message): return {"node_name": self.root.nodename, "status": "error", "message": message} From 19ed1eb3e36b55948734160f061848da2db31c5f Mon Sep 17 00:00:00 2001 From: James McKinney <26463+jpmckinney@users.noreply.github.com> Date: Tue, 16 Jul 2024 17:12:19 -0400 Subject: [PATCH 13/13] ci: Skip kill (%% doesn't work on Windows) --- .github/workflows/tests.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index ddda0810..34d5c272 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -25,7 +25,6 @@ jobs: sleep 1 pytest integration_tests cat scrapyd.log - kill %% - env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: coveralls --service=github