search

scrapy-plugins / scrapy-zyte-api

Zyte API integration for Scrapy
BSD 3-Clause "New" or "Revised" License
36 stars 19 forks source link

python-zyte-api 0.5.0 support #185

Closed Gallaecio closed 7 months ago

codecov[bot] commented 7 months ago

Codecov Report

Merging #185 (218a184) into main (2bfb2be) will decrease coverage by 1.43%. The diff coverage is 100.00%.

:exclamation: Current head 218a184 differs from pull request most recent head d09e3f3. Consider uploading reports for the commit d09e3f3 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #185 +/- ## ========================================== - Coverage 98.57% 97.14% -1.43% ========================================== Files 13 13 Lines 1052 1052 ========================================== - Hits 1037 1022 -15 - Misses 15 30 +15 ``` | [Files](https://app.codecov.io/gh/scrapy-plugins/scrapy-zyte-api/pull/185?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scrapy-plugins) | Coverage Δ | | |---|---|---| | [scrapy\_zyte\_api/handler.py](https://app.codecov.io/gh/scrapy-plugins/scrapy-zyte-api/pull/185?src=pr&el=tree&filepath=scrapy_zyte_api%2Fhandler.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scrapy-plugins#diff-c2NyYXB5X3p5dGVfYXBpL2hhbmRsZXIucHk=) | `98.26% <100.00%> (ø)` | | ... and [2 files with indirect coverage changes](https://app.codecov.io/gh/scrapy-plugins/scrapy-zyte-api/pull/185/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scrapy-plugins)
Gallaecio commented 7 months ago

Some tests are hanging with the new API. I suspect it has to do not with the new API, but with making engine_started async. But I’m having a hard time figuring it out, so I’ll come back later with fresh eyes (if anyone wants to give it a go before that, be my guess).

Gallaecio commented 7 months ago

It seems the issue was that open_spider ended up being called before the engine started. And the engine_started signal also fires before engine.running is set to True. So the earliest point I could think of to close the spider, which requires the engine to be running, was to wait until the first request processing.

Gallaecio commented 7 months ago

Test failures are unrelated to this change.