Skip to content

Extend Request.meta documentation #5565

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 3 commits into from
Nov 30, 2023
Merged

Conversation

Gallaecio
Copy link
Member

While reviewing #4141, I realized the existing Request.meta documentation could use some more information.

@codecov
Copy link

codecov bot commented Jul 19, 2022

Codecov Report

Merging #5565 (f8709a1) into master (fa690fb) will increase coverage by 0.15%.
The diff coverage is n/a.

❗ Current head f8709a1 differs from pull request most recent head e76fd8b. Consider uploading reports for the commit e76fd8b to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5565      +/-   ##
==========================================
+ Coverage   88.59%   88.75%   +0.15%     
==========================================
  Files         159      162       +3     
  Lines       11582    10774     -808     
  Branches     1885     1843      -42     
==========================================
- Hits        10261     9562     -699     
+ Misses        994      939      -55     
+ Partials      327      273      -54     

see 156 files with indirect coverage changes

:attr:`cb_kwargs` instead.

To keep some data across 3 or more spider callbacks, however, request
metadata may be the right choice.
Copy link
Member

Choose a reason for hiding this comment

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

Hm, I think cb_kwargs also works ok in this case - you can pass something like partial_data dict or list as a kwarg to all callbacks

Copy link
Member Author

@Gallaecio Gallaecio Jul 20, 2022

Choose a reason for hiding this comment

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

The use case I am trying to describe here is where you pass some specific metadata key to almost every callback, and new future callbacks are likely to get it as well, but often most callbacks only pass it over to the next callback. So, for example, start_url if you want to keep track of the root URL that originated every callback.

In those cases, adding the same parameter to every callback, specially to callbacks whose only function is to pass the parameter over to the next callback, may be too verbose, and using meta instead (and eventually STICKY_META_KEYS, #4141, which is what inspired this paragraph) may be better.

I do need to come up with a better wording. Let me give it at least 1 more try.

Copy link
Member

@kmike kmike left a comment

Choose a reason for hiding this comment

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

👍 I left a comment, but feel free to merge without addressing it.

@kmike kmike added this to the 2.7 milestone Jul 27, 2022
@kmike kmike removed this from the 2.7 milestone Oct 13, 2022
@wRAR wRAR added this to the Scrapy 2.10 milestone May 4, 2023
@wRAR wRAR modified the milestones: Scrapy 2.10, Scrapy 2.11 Aug 7, 2023
@Gallaecio Gallaecio merged commit 731f749 into scrapy:master Nov 30, 2023
wRAR pushed a commit that referenced this pull request Dec 20, 2023
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.

3 participants