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 missing storage.store calls in FeedExporter.close_spider #4626

Merged
merged 1 commit into from Jun 17, 2020

Conversation

StasDeep
Copy link
Contributor

@StasDeep StasDeep commented Jun 11, 2020

Fixes #4621.

@codecov
Copy link

codecov bot commented Jun 11, 2020

Codecov Report

Merging #4626 into master will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #4626      +/-   ##
==========================================
+ Coverage   84.76%   84.79%   +0.02%     
==========================================
  Files         163      163              
  Lines        9978     9987       +9     
  Branches     1489     1487       -2     
==========================================
+ Hits         8458     8468      +10     
  Misses       1254     1254              
+ Partials      266      265       -1     
Impacted Files Coverage Δ
scrapy/extensions/feedexport.py 84.61% <100.00%> (+0.14%) ⬆️
scrapy/utils/trackref.py 82.85% <0.00%> (-2.86%) ⬇️
scrapy/shell.py 67.96% <0.00%> (-0.25%) ⬇️
scrapy/loader/__init__.py 95.54% <0.00%> (-0.03%) ⬇️
scrapy/exporters.py 100.00% <0.00%> (ø)
scrapy/core/scraper.py 87.97% <0.00%> (ø)
scrapy/commands/parse.py 20.21% <0.00%> (ø)
scrapy/utils/serialize.py 93.54% <0.00%> (ø)
scrapy/contracts/default.py 84.61% <0.00%> (+0.30%) ⬆️
scrapy/pipelines/files.py 60.64% <0.00%> (+0.71%) ⬆️
... and 1 more

@StasDeep
Copy link
Contributor Author

StasDeep commented Jun 11, 2020

@elacuesta could you please review it?

@elacuesta
Copy link
Member

elacuesta commented Jun 13, 2020

Thanks for the patch. I will try to review it next week.

tests/test_feedexport.py Outdated Show resolved Hide resolved
tests/test_feedexport.py Outdated Show resolved Hide resolved
@StasDeep StasDeep force-pushed the fix/stores-only-one-feed branch from 75d361e to 4e5d88b Compare Jun 15, 2020
Copy link
Member

@Gallaecio Gallaecio left a comment

Looks good to me, thanks!

PS: 💄 I believe that print(log) line from the test can be removed. #4629 (comment)

Copy link
Member

@elacuesta elacuesta left a comment

Good catch, I definitely missed this in the transition to multiple feeds. Thanks!

@Gallaecio Gallaecio merged commit 3d027fb into scrapy:master Jun 17, 2020
2 checks passed
alex-ber added a commit to alex-ber/scrapy-feedstreaming that referenced this issue Jul 2, 2020
alex-ber added a commit to alex-ber/scrapy-feedstreaming that referenced this issue Jul 12, 2020
### Added
* Buffering was added to `item_scraped()`.
* S3FeedStorage: you can specify `ACL` as query part of URI.
* S3FeedStorage: support of `region` is added.
* FEEDS: `slot_key_param`: New (not available in Scrapy itself) specify (global) function which takes item and spider as parameter
and `slot_key`. Given the item that is passed through the pipeline to what URI you want to send it.
Fall back to noop method — method that does nothing.
* FEEDS: `buff_capacity`: New (not available in Scrapy itself) — after what amount of item you want to export them.
The fall back value is 1.
* `_FeedSlot` instances are created from your settings. They are created per supplied URI.
Some extra (compare to Scraping) information is stored, namely:
- `uri_template` — it is available through public API get_slots() method, see below.
- `spider_name` — is used in public API get_slots() method to restrict returned slots for requested spider.
- `buff_capacity` —buffer’s capacity, if the number of item exceed this number the buffer is flushed
- `buff` — buffer where all items pending export are stored.
* `FeedExported` there is 1 extra public method
- `get_slots()` — this method is used to get feed slot’s information (see implementation note above). It is populated from the settings. For example, you can retrieve to either URI you will export the items.
Note:
1. `slot_key` is slot identifier as described above. If you have only 1 URI you can supply None for this value.
2. You can retrieve feed slot’s information only from your spider.
3. It has optional `force_create=True` parameter.
If you’re calling this method early in the Scrapy life-cycle feed slot’s information may be not yet created.
In this case, the default behavior is to create this information and return it for you.
If `force_create=False` is supplied you will receive an empty collection of feed slot’s information.
* On `S3FeedStorage` there couple of public methods:

- `botocore_session`
- `botocore_client`
- `botocore_base_kwargs` — dict of minimal parameters for `botocore_client.put_object()` method as supplied in settings.
- `botocore_kwargs` — dict of all supplied parameters `for botocore_client.put_object()` method as supplied in settings.
For example, if supplied, it will contain `ACL` parameter while `botocore_base_kwargs` will not contain it.

### Changed
* You can have multiple URI for exports.
* Logic of sending the item was moved from the `close_spider()` to `item_scraped()`.
* back-port Fix missing `storage.store()` calls in `FeedExporter.close_spider()` [scrapy/scrapy#4626]
* back-port Fix duplicated feed logs [scrapy/scrapy#4629]

### Removed
* removed deprecated: fallback to `boto` library if `botocore` is not found
* removed deprecated: implicit retrieval of settings from the project — settings is passed explicitly now
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.

3 participants