feat: Unify Apify and Scrapy to use single event loop & remove `nest-asyncio` #390

vdusek · 2025-02-06T14:43:40Z

Description

Apify (asyncio) and Scrapy (Twisted) now run on a single event loop.
- nest-asyncio has been completely removed.
- It seems that this change also improved the performance.
The ApifyScheduler, which is synchronous, now executes asyncio coroutines (communication with RQ) in a separate thread with its own asyncio event loop.
Logging setup has to be adjusted and I moved to a dedicated file in the SDK.
The try-import functionality for optional dependecies from Crawlee was added to scrapy subpackage.
A new integration test for Scrapy Actor has been added.

Issues

Closes: Add integration test for Scrapy integration #148
Closes: Move most of the Scrapy template's logic to Apify SDK #176
Closes: Request queue stuck in infinite loop due to ID mismatch in Apify-Scrapy integration #392
Relates: Scrapy template doesn't handle imminent migration to another host actor-templates#303
- This issue will be closed once the corresponding PR in actor-templates is merged.

Tests

A new integration test for Scrapy Actor has been added.
And of course, it was tested manually using the Actor from guides/templates.

Next steps

Update Scrapy Actor template in actor-templates.
Update Actor Scrapy Books Example.
Add HTTP cache storage for KVS, @honzajavorek will provide his implementation.

Follow-up issues

There are still a few issues to be resolved.
Duplicate logs from apify logger in Scrapy-Apify integration #391
Request deduplication does not work in Apify-Scrapy integration #395

honzajavorek

Very nice beautiful! I scanned the changes and they look like the stuff I've been through for the past two weeks. I didn't spot anything suspicious except two things where I commented.

This wasn't a thorough review of all the lines, so I'm just "commenting", not approving or disapproving.

.github/workflows/run_code_checks.yaml

docs/02_guides/code/scrapy_project/src/main.py

docs/02_guides/code/scrapy_project/src/__main__.py

docs/02_guides/code/scrapy_project/src/main.py

docs/02_guides/05_scrapy.mdx

docs/02_guides/code/scrapy_project/src/spiders/title.py

src/apify/scrapy/scheduler.py

Closes: #392

janbuchar

Solid job! I didn't manage to read the whole thing yet, here's a bunch of random comments.

tests/integration/test_actor_scrapy.py

src/apify/_actor.py

tests/integration/conftest.py

Pijukatel · 2025-02-11T12:57:13Z

Please merge from latest master to your branch and double check that the code in examples is not longer than 90.

vdusek · 2025-02-11T13:17:02Z

Please merge from latest master to your branch and double check that the code in examples is not longer than 90.

@Pijukatel So please provide the same PR as for the Crawlee (apify/crawlee-python#973) here and to the Apify Python client as well 🙂.

Pijukatel · 2025-02-11T13:21:37Z

Please merge from latest master to your branch and double check that the code in examples is not longer than 90.

@Pijukatel So please provide the same PR as for the Crawlee (apify/crawlee-python#973) here and to the Apify Python client as well 🙂.

Haha, I see this is different repo. I will prepare the PR for this repo as well and merge it after you to not block you.

janbuchar

Two documentation-related notes, otherwise it looks good!

janbuchar · 2025-02-12T16:29:56Z

src/apify/_actor.py

@@ -270,8 +270,8 @@ async def finalize() -> None:
            self.log.debug(f'Not calling sys.exit({exit_code}) because Actor is running in IPython')
        elif os.getenv('PYTEST_CURRENT_TEST', default=False):  # noqa: PLW1508
            self.log.debug(f'Not calling sys.exit({exit_code}) because Actor is running in an unit test')
-        elif hasattr(asyncio, '_nest_patched'):
-            self.log.debug(f'Not calling sys.exit({exit_code}) because Actor is running in a nested event loop')
+        elif os.getenv('SCRAPY_SETTINGS_MODULE'):


Please mention that we now depend on this to be present in the upgrading guide, just to be sure.

I am not sure about the upgrading guide mentioning minor versions. Is it okay if I add it to the Scrapy guide doc page?

Edit: it is there

janbuchar · 2025-02-12T16:38:12Z

docs/02_guides/05_scrapy.mdx


-The fastest way to start using Scrapy in Apify Actors is by leveraging the [Scrapy Actor template](https://apify.com/templates/categories/python). This template provides a pre-configured structure and setup necessary to integrate Scrapy into your Actors seamlessly. It includes: setting up the Scrapy settings, `asyncio` reactor, Actor logger, and item pipeline as necessary to make Scrapy spiders run in Actors and save their outputs in Apify datasets.
+## Key integration components


It would be awesome if we could have a nice diagram here 🙂 In the ideal case, it would show how the asyncio event loop and twisted reactor interact. We can definitely postpone that though.

The point is that Asyncio and Twisted share the same event loop. I'm not familiar with the details of how Asyncio and Twisted exactly interact together. Other than that, we have a separate thread with an Asyncio event loop for the Scheduler's synchronous calls.

We gave it two weeks and made it work, but that doesn't mean we understand the sorcery enough to draw diagrams 😅 But I think it's something like...

(It's black on transparent SVG, so you probably won't see it well in dark mode)

I don't know if I chose the best diagram for this, maybe https://swimlanes.io/ would be better

honzajavorek · 2025-02-13T13:47:02Z

src/apify/scrapy/scheduler.py

+            logger.exception('Exception occurred while shutting down.')
+
+        finally:
+            logger.debug(f'{self.__class__.__name__} closed successfully.')


Successfully 🤔 I think inside finally we shouldn't say that word.

github-actions bot assigned vdusek Feb 6, 2025

github-actions bot added this to the 107th sprint - Tooling team milestone Feb 6, 2025

github-actions bot added the t-tooling Issues with this label are in the ownership of the tooling team. label Feb 6, 2025

vdusek changed the title ~~fix: fix Scrapy integration~~ fix: Unify Apify and Scrapy to use single event loop & remove nest-asyncio Feb 6, 2025

vdusek had a problem deploying to fork-pr-integration-tests February 6, 2025 15:35 — with GitHub Actions Error

vdusek force-pushed the fixing-scrapy branch from 1e37e47 to c33b83f Compare February 6, 2025 18:27

github-actions bot added the tested Temporary label used only programatically for some analytics. label Feb 7, 2025

vdusek requested review from janbuchar and Pijukatel February 7, 2025 11:41

vdusek marked this pull request as ready for review February 7, 2025 11:41

honzajavorek reviewed Feb 7, 2025

View reviewed changes

.github/workflows/run_code_checks.yaml Outdated Show resolved Hide resolved

docs/02_guides/code/scrapy_project/src/main.py Show resolved Hide resolved

Pijukatel reviewed Feb 10, 2025

View reviewed changes

vdusek requested a review from Pijukatel February 10, 2025 13:48

vdusek added 9 commits February 10, 2025 14:52

fix: fix Scrapy integration

8d40e40

comment out docs check CI step

8cc38b4

Scrapy integration test is working

c1eacba

rm scrapy condition for sys exit

bd50787

Polishment

0c0fbcc

revert non-intentionally changes in proxy conf

357cc27

Update the Scrapy guide

e7f0aeb

add async thread helper class

1321f82

address feedback

f1a7fd7

vdusek force-pushed the fixing-scrapy branch from 197026c to f1a7fd7 Compare February 10, 2025 13:53

polishment

f99a9eb

Pijukatel approved these changes Feb 10, 2025

View reviewed changes

vdusek added 2 commits February 10, 2025 16:52

do not run sys.exit for scrapy

7f2242d

Fix RQ stuck in infinite loop due to ID mismatch

57086f5

Closes: #392

vdusek modified the milestones: 107th sprint - Tooling team, 108th sprint - Tooling team Feb 10, 2025

janbuchar reviewed Feb 10, 2025

View reviewed changes

tests/integration/test_actor_scrapy.py Outdated Show resolved Hide resolved

src/apify/_actor.py Show resolved Hide resolved

src/apify/_actor.py Outdated Show resolved Hide resolved

tests/integration/conftest.py Outdated Show resolved Hide resolved

janbuchar self-requested a review February 10, 2025 17:43

vdusek added 2 commits February 11, 2025 09:22

Address Honza's feedback

8646c06

utilize SCRAPY_SETTINGS_MODULE env var

4a9665a

allow ajax crawl middleware

f1e8bd5

vdusek force-pushed the fixing-scrapy branch from e785bf1 to f1e8bd5 Compare February 11, 2025 15:00

vdusek changed the title ~~fix: Unify Apify and Scrapy to use single event loop & remove nest-asyncio~~ feat: Unify Apify and Scrapy to use single event loop & remove nest-asyncio Feb 12, 2025

janbuchar requested changes Feb 12, 2025

View reviewed changes

mention SCRAPY_SETTINGS_MODULE env var

413a56a

janbuchar self-requested a review February 13, 2025 09:06

janbuchar approved these changes Feb 13, 2025

View reviewed changes

vdusek merged commit 96949be into master Feb 13, 2025
23 of 26 checks passed

vdusek deleted the fixing-scrapy branch February 13, 2025 09:07

honzajavorek mentioned this pull request Feb 13, 2025

Figuring out nested asyncio loops juniorguru/plucker#108

Merged

15 tasks

honzajavorek reviewed Feb 13, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Unify Apify and Scrapy to use single event loop & remove `nest-asyncio` #390

feat: Unify Apify and Scrapy to use single event loop & remove `nest-asyncio` #390

vdusek commented Feb 6, 2025 •

edited

Loading

honzajavorek left a comment •

edited

Loading

janbuchar left a comment

Pijukatel commented Feb 11, 2025 •

edited

Loading

vdusek commented Feb 11, 2025

Pijukatel commented Feb 11, 2025

janbuchar left a comment

janbuchar Feb 12, 2025

vdusek Feb 12, 2025 •

edited

Loading

janbuchar Feb 12, 2025

vdusek Feb 12, 2025

honzajavorek Feb 13, 2025

honzajavorek Feb 13, 2025

honzajavorek Feb 13, 2025


		The fastest way to start using Scrapy in Apify Actors is by leveraging the [Scrapy Actor template](https://apify.com/templates/categories/python). This template provides a pre-configured structure and setup necessary to integrate Scrapy into your Actors seamlessly. It includes: setting up the Scrapy settings, `asyncio` reactor, Actor logger, and item pipeline as necessary to make Scrapy spiders run in Actors and save their outputs in Apify datasets.
		## Key integration components

feat: Unify Apify and Scrapy to use single event loop & remove nest-asyncio #390

feat: Unify Apify and Scrapy to use single event loop & remove nest-asyncio #390

Conversation

vdusek commented Feb 6, 2025 • edited Loading

Description

Issues

Tests

Next steps

Follow-up issues

honzajavorek left a comment • edited Loading

Choose a reason for hiding this comment

janbuchar left a comment

Choose a reason for hiding this comment

Pijukatel commented Feb 11, 2025 • edited Loading

vdusek commented Feb 11, 2025

Pijukatel commented Feb 11, 2025

janbuchar left a comment

Choose a reason for hiding this comment

janbuchar Feb 12, 2025

Choose a reason for hiding this comment

vdusek Feb 12, 2025 • edited Loading

Choose a reason for hiding this comment

janbuchar Feb 12, 2025

Choose a reason for hiding this comment

vdusek Feb 12, 2025

Choose a reason for hiding this comment

honzajavorek Feb 13, 2025

Choose a reason for hiding this comment

honzajavorek Feb 13, 2025

Choose a reason for hiding this comment

honzajavorek Feb 13, 2025

Choose a reason for hiding this comment

feat: Unify Apify and Scrapy to use single event loop & remove `nest-asyncio` #390

feat: Unify Apify and Scrapy to use single event loop & remove `nest-asyncio` #390

vdusek commented Feb 6, 2025 •

edited

Loading

honzajavorek left a comment •

edited

Loading

Pijukatel commented Feb 11, 2025 •

edited

Loading

vdusek Feb 12, 2025 •

edited

Loading