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

Allow overwriting feeds #4512

Merged
merged 53 commits into from Aug 17, 2020
Merged

Allow overwriting feeds #4512

merged 53 commits into from Aug 17, 2020

Conversation

Gallaecio
Copy link
Member

@Gallaecio Gallaecio commented Apr 24, 2020

This is a fork of #716.

I’ve aimed for backward-compatible behavior (hence overwrite has a different default value for the local filesystem and FTP storages). Still, there is a backward-incompatible change to IFeedStorage.__init__; on the bright side, it will fail loudly and be easy to fix (add the new parameter to __init__ and ignore it otherwise).

On FTP, in addition to adding support for APPE operations, I:

  • Fixed a bug when a password is not specified (hence None, failing when passed to unquote).
  • Included a mock FTP server in the tests, so that they always run. I wanted to use Twisted, but its FTP server does not support APPE operations.

To do:

  • Fix tests

Fixes #547, closes #716.

@Gallaecio Gallaecio changed the title Allow overwriting feeds [WIP] Allow overwriting feeds Apr 24, 2020
@codecov
Copy link

codecov bot commented Apr 27, 2020

Codecov Report

Merging #4512 into master will increase coverage by 0.38%.
The diff coverage is 93.42%.

@@            Coverage Diff             @@
##           master    #4512      +/-   ##
==========================================
+ Coverage   86.79%   87.17%   +0.38%     
==========================================
  Files         160      160              
  Lines        9754     9795      +41     
  Branches     1433     1442       +9     
==========================================
+ Hits         8466     8539      +73     
+ Misses       1025      995      -30     
+ Partials      263      261       -2     
Impacted Files Coverage Δ
scrapy/extensions/feedexport.py 95.01% <90.90%> (+5.29%) ⬆️
scrapy/commands/__init__.py 67.14% <100.00%> (+4.82%) ⬆️
scrapy/utils/conf.py 93.69% <100.00%> (+0.55%) ⬆️
scrapy/utils/ftp.py 78.26% <100.00%> (+54.45%) ⬆️
scrapy/utils/python.py 85.79% <100.00%> (+0.08%) ⬆️
scrapy/utils/trackref.py 85.71% <0.00%> (+2.85%) ⬆️
scrapy/commands/crawl.py 60.00% <0.00%> (+25.00%) ⬆️

@Gallaecio Gallaecio changed the title [WIP] Allow overwriting feeds Allow overwriting feeds Apr 27, 2020
@kmike
Copy link
Member

kmike commented May 3, 2020

@Gallaecio could you please check the tutorial - can it be updated with this feature? See https://docs.scrapy.org/en/latest/intro/tutorial.html#storing-the-scraped-data

docs/topics/feed-exports.rst Outdated Show resolved Hide resolved
docs/intro/tutorial.rst Show resolved Hide resolved
scrapy/commands/crawl.py Outdated Show resolved Hide resolved
scrapy/utils/conf.py Outdated Show resolved Hide resolved
scrapy/utils/conf.py Show resolved Hide resolved
scrapy/utils/conf.py Outdated Show resolved Hide resolved
docs/topics/feed-exports.rst Outdated Show resolved Hide resolved
docs/topics/feed-exports.rst Outdated Show resolved Hide resolved
docs/topics/feed-exports.rst Outdated Show resolved Hide resolved
scrapy/extensions/feedexport.py Outdated Show resolved Hide resolved
scrapy/extensions/feedexport.py Outdated Show resolved Hide resolved
@Gallaecio Gallaecio changed the title Allow overwriting feeds [WIP] Allow overwriting feeds Jun 15, 2020
@Gallaecio Gallaecio changed the title [WIP] Allow overwriting feeds Allow overwriting feeds Jun 15, 2020
@Gallaecio
Copy link
Member Author

Gallaecio commented Jun 15, 2020

So, the tests were failing due to unclosed socket warnings randomly getting caught by the new tests. Since the issue is not easily reproducible, I’ve taken the approach to ignore those warnings in the tests that inspect warning content, and only take into account Scrapy deprecation warnings.

wRAR
wRAR approved these changes Jul 16, 2020
@Gallaecio Gallaecio merged commit e70975f into scrapy:master Aug 17, 2020
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a command-line option for overwriting exported file
5 participants