Skip to content

Fix Feed Exporter issue with Post Processing #5581

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

Merged
merged 6 commits into from
Apr 11, 2023

Conversation

felipeboffnunes
Copy link
Member

@felipeboffnunes felipeboffnunes commented Jul 28, 2022

Fixes #5500

It fixes the issue above. It happens because when dealing with Post Processing, file is wrapped around a PostProcessingManager, which becomes the *arg for the following procedures that will happen in FeedExporter. Those procedures believe they are handling directly the file: this is not the case when Post Processing is involved, since the file becomes an attribute of the PostProcessingManager in these cases.

We need to test this, but it's a very straightforward bug nevertheless.

@felipeboffnunes
Copy link
Member Author

This is under discussion with @Gallaecio.

@Gallaecio
Copy link
Member

Proposed test: felipeboffnunes#1

@codecov
Copy link

codecov bot commented Apr 10, 2023

Codecov Report

Merging #5581 (500aaa2) into master (d60b4ed) will increase coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head 500aaa2 differs from pull request most recent head 23017e6. Consider uploading reports for the commit 23017e6 to get more accurate results

@@            Coverage Diff             @@
##           master    #5581      +/-   ##
==========================================
+ Coverage   88.84%   88.86%   +0.01%     
==========================================
  Files         162      162              
  Lines       11055    11062       +7     
  Branches     1800     1802       +2     
==========================================
+ Hits         9822     9830       +8     
  Misses        954      954              
+ Partials      279      278       -1     
Impacted Files Coverage Δ
scrapy/extensions/feedexport.py 95.52% <100.00%> (+0.07%) ⬆️
scrapy/utils/ssl.py 57.50% <100.00%> (+2.23%) ⬆️

... and 1 file with indirect coverage changes

Copy link
Member

@Gallaecio Gallaecio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

@wRAR wRAR merged commit bdb78b9 into scrapy:master Apr 11, 2023
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.

Postprocessing feeds do not work for S3 feed storage
3 participants