Skip to content
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

Fix (at least) windows problem. #19

Merged
merged 1 commit into from
Nov 11, 2013
Merged

Fix (at least) windows problem. #19

merged 1 commit into from
Nov 11, 2013

Conversation

lookfwd
Copy link
Contributor

@lookfwd lookfwd commented Jul 29, 2013

When you run scrapyd on Windows, paths like C:\blah are generated and then the "C" is assumed to be the storage type resulting in errors ("Unknown feed storage scheme: c" - from feedexport.py). Adding "file://" should be enough to fix this problem.

When you run scrapyd on Windows, paths like C:\\blah are generated and then the "C" is assumed to be the storage type resulting in errors ("Unknown feed storage scheme: c" - from feedexport.py). Adding "file://" should be enough to fix this problem.
@pablohoffman
Copy link
Member

@lookfwd thanks for helping to improve Scrapyd on windows, it's a platform traditionally not well supported on Scrapyd (unlike Scrapy), so expect more simple fixes like this.

I would like to merge this change but at the same time have it tested to make sure it won't break again, any ides on how to do that?. Maybe we could port the extras/text-scrapyd.sh to windows.

@lookfwd
Copy link
Contributor Author

lookfwd commented Sep 4, 2013

Thanks, actually I do some cross-testing between Windows and Linux etc for the book. The only other limitation I found is the length of Arguments on the exec of the spider which is limited in comparison to linux. It's probably just a documentation issue. I will have a look on extras/text-scrapyd.sh

pablohoffman added a commit that referenced this pull request Nov 11, 2013
Fix (at least) windows problem.
@pablohoffman pablohoffman merged commit 599a605 into scrapy:master Nov 11, 2013
@pablohoffman
Copy link
Member

@lookfwd I decided to merged this one, but I look forward to improving the test suite (esp. on windows).

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.

2 participants