Skip to content

Add process_spider_output_async() to the spider middleware. #91

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

wRAR
Copy link
Member

@wRAR wRAR commented Dec 27, 2024

Fixes #72.

I wonder how can we test the change on Scrapy Cloud before merging this though?

@Gallaecio
Copy link
Member

Gallaecio commented Dec 27, 2024

I wonder how can we test the change on Scrapy Cloud before merging this though?

I believe you can install a newer version of the entrypoint through requirements.txt, the only thing that does not update properly is the execution of binaries I think.

@@ -1,5 +1,6 @@
# -*- coding: utf-8 -*-
import itertools
from warnings import warn
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for the call in HubstorageDownloaderMiddleware.from_crawler(), added in some older change.

@@ -28,11 +29,22 @@ def process_spider_output(self, response, result, spider):
parent = self._seen_requests.pop(response.request, None)
for x in result:
if isinstance(x, Request):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check not extracted because we tentatively decided that as a general pattern for such middlewares we prefer to separate "process_request" and "process_item" logic.

@wRAR
Copy link
Member Author

wRAR commented Jan 6, 2025

I confirmed this works (if the project has an async middleware, there is a warning normally and no warning if this branch is installed explicitly).

@kmike
Copy link
Member

kmike commented Jan 21, 2025

@wRAR it seems Python 3.7 tests are failing. Do you know if these are CI failures, or if the code actually doesn't work?
It seems we should drop Python 3.7 support from the entrypoint, and ensure 3.7 stacks don't get a wrong entrypoint version. But probably it'd be better to do it separately, if the code still works.

@wRAR
Copy link
Member Author

wRAR commented Jan 21, 2025

@kmike the Ubuntu 24.04 runner doesn't have 3.7 and earlier, it's a known limitation. I think the suggested way to fix that is pinning 22.04 for such jobs.

@wRAR
Copy link
Member Author

wRAR commented Feb 12, 2025

I submitted #92 that includes a fix for the 3.7 CI job.

@wRAR wRAR closed this Feb 12, 2025
@wRAR wRAR reopened this Feb 12, 2025
Copy link

codecov bot commented Feb 12, 2025

Codecov Report

Attention: Patch coverage is 0% with 11 lines in your changes missing coverage. Please review.

Project coverage is 23.62%. Comparing base (970b45c) to head (0c1cb20).

Files with missing lines Patch % Lines
sh_scrapy/middlewares.py 0.00% 11 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master      #91       +/-   ##
===========================================
- Coverage   92.24%   23.62%   -68.62%     
===========================================
  Files          15       15               
  Lines         812      821        +9     
===========================================
- Hits          749      194      -555     
- Misses         63      627      +564     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@wRAR
Copy link
Member Author

wRAR commented Mar 21, 2025

Hi @elacuesta ! We are close to releasing the new Scrapy version and unless this change is included in the updated stack there will be warnings about this middleware in the spider logs so can we please proceed with reviewing and merging this in the next several weeks? Thank you!

@wRAR wRAR closed this Mar 21, 2025
@wRAR wRAR reopened this Mar 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Spider middlewares are not async compatible
3 participants