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 duplicated feed logs #4629

Merged
merged 1 commit into from Jun 23, 2020
Merged

Conversation

StasDeep
Copy link
Contributor

@StasDeep StasDeep commented Jun 13, 2020

Fixes #4619.

@codecov
Copy link

codecov bot commented Jun 13, 2020

Codecov Report

Merging #4629 into master will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #4629      +/-   ##
==========================================
+ Coverage   84.80%   84.81%   +0.01%     
==========================================
  Files         163      163              
  Lines        9987     9987              
  Branches     1487     1487              
==========================================
+ Hits         8469     8470       +1     
  Misses       1254     1254              
+ Partials      264      263       -1     
Impacted Files Coverage Δ
scrapy/extensions/feedexport.py 85.06% <100.00%> (+0.45%) ⬆️

@StasDeep StasDeep force-pushed the fix/duplicated-feed-logs branch from 9cd97ae to c910429 Compare Jun 15, 2020
@StasDeep
Copy link
Contributor Author

StasDeep commented Jun 15, 2020

@Gallaecio thanks for the review! I've addressed your comments.

Copy link
Member

@Gallaecio Gallaecio left a comment

Great!

💄 I think those print(log) lines in the tests are debug leftovers that you meant to remove 🙂

@StasDeep
Copy link
Contributor Author

StasDeep commented Jun 16, 2020

@Gallaecio thanks.
Well, I've seen print(log) in other log-based tests and thought that it is done to show the log in case of failure.

@StasDeep StasDeep force-pushed the fix/duplicated-feed-logs branch from c910429 to a4bfd5a Compare Jun 19, 2020
@StasDeep
Copy link
Contributor Author

StasDeep commented Jun 19, 2020

@Gallaecio should I summon anyone else for review?

@Gallaecio
Copy link
Member

Gallaecio commented Jun 22, 2020

It may take a few weeks before someone else finds time to review it, so please be patient. I don’t think we will merge it in time for the upcoming 2.2 release, but I’m hoping it will make it to 2.3.

@StasDeep
Copy link
Contributor Author

StasDeep commented Jun 22, 2020

@Gallaecio thanks for letting me now.

wRAR
wRAR approved these changes Jun 23, 2020
@wRAR wRAR merged commit 536643e into scrapy:master Jun 23, 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