-
Notifications
You must be signed in to change notification settings - Fork 457
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
Review url being optional for SplashRequest #270
Comments
sounds good to me, thanks for the heads up @Gallaecio |
Reverting 4154782 will do just that, so just go for it. |
Hi. Is this issue still relevant to the latest version of scrapy-splash? I reverted the changes in 4154782, ran the tests locally and no failures were noticed. |
Well, I think reverting that commit is precisely the goal. I assume you reverted the entire commit, nor just the non-test code, and that keeping the current test code and applying the rest of the commit would result in broken tests. |
Yes, I reverted the entire commit including the changes to the tests. |
My understanding is that, reverting the commit, scrapy-splash would fail with scrapy>=2,<2.4 if the URL is omitted, because the default URL uses the unsupported The easiest solution I think would be to revert that commit and require scrapy>=2.4. |
As part of #269, the
url
parameter toSplashRequest
is no longer optional.@elacuesta noticed that this is a backward-incompatible change. Moreover, the upcoming Scrapy 2.4 will make this change unnecessary (scrapy/scrapy#4835).
We should consider reverting that part of #269.
The text was updated successfully, but these errors were encountered: