Handling CloseSpider exception if it has been raised in start_requests() #6148
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #6148 +/- ##
==========================================
+ Coverage 88.55% 88.63% +0.07%
==========================================
Files 159 159
Lines 11582 11593 +11
Branches 1885 1887 +2
==========================================
+ Hits 10257 10276 +19
+ Misses 996 990 -6
+ Partials 329 327 -2
|
|
Still working on it, got failed tests after some changes. |
|
@Gallaecio @wRAR could you please take a look if have time? |
| if self.slot is not None: | ||
| raise RuntimeError("Engine slot is already assigned. Use self.close_spider") | ||
|
|
||
| self.start() |
There was a problem hiding this comment.
This doesn't look correct because start() returns a Deferred, but also is it OK to call it here at all if we are closing anyway?
There was a problem hiding this comment.
@wRAR
I didn't place it at beginning. But to common close_spider method has several checks for engine being started. Not sure we need to have use it though, just think that the code inside it is useful.
I've tried to avoid it, but neither of approaches I've considered allowed to get the proper result. The only other option I see so far is to simply set close reason to spider and leave it in piece.
How do you think to do it better?
There was a problem hiding this comment.
In any case if you call it you need to wait until it completes.
Proposal for issue #3463.
I assume it's not the cleanest approach, so I'm opened for change requests after review.
I was trying to modify the
ExecutionEngine.open_spiderin a way that thestart_requests's exception would be called inside it. But it looked that it would take too much changes, so a lot of stuff can be broken.Still, I'm opened to any ideas of how to make this change better.