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

Cleanup deprecated fingerprint code in scrapy.utils.request #6213

Merged
merged 6 commits into from
Jan 31, 2024
Merged

Cleanup deprecated fingerprint code in scrapy.utils.request #6213

merged 6 commits into from
Jan 31, 2024

Conversation

Laerte
Copy link
Member

@Laerte Laerte commented Jan 31, 2024

Closes #6212

Copy link

codecov bot commented Jan 31, 2024

Codecov Report

Merging #6213 (53ccf00) into master (4229048) will decrease coverage by 0.07%.
The diff coverage is 75.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6213      +/-   ##
==========================================
- Coverage   88.62%   88.55%   -0.07%     
==========================================
  Files         160      160              
  Lines       11638    11607      -31     
  Branches     1892     1883       -9     
==========================================
- Hits        10314    10279      -35     
- Misses        999     1003       +4     
  Partials      325      325              
Files Coverage Δ
scrapy/settings/default_settings.py 98.80% <100.00%> (ø)
scrapy/utils/test.py 84.81% <ø> (-0.38%) ⬇️
scrapy/utils/request.py 90.09% <66.66%> (-5.29%) ⬇️

@Gallaecio
Copy link
Member

My plan here was to deprecate REQUEST_FINGERPRINTER_IMPLEMENTATION itself now, and remove the setting altogether in a year.

@Laerte
Copy link
Member Author

Laerte commented Jan 31, 2024

My plan here was to deprecate REQUEST_FINGERPRINTER_IMPLEMENTATION itself now, and remove the setting altogether in a year.

Should i add a deprecation warning to RequestFingerprinter constructor then?

@Gallaecio
Copy link
Member

Gallaecio commented Jan 31, 2024

I’m open when it comes to how to implement that, but one way to go about it would be to switch the default to some sentinel value (e.g. "SENTINEL"), and log a deprecation message if the value is something else, asking for the removal of the setting altogether.

And we should stop adding the setting to settings.py when using startproject.

And we can entirely remove any mention of the setting from the docs.

scrapy/utils/request.py Outdated Show resolved Hide resolved
scrapy/utils/test.py Outdated Show resolved Hide resolved
@Laerte Laerte self-assigned this Jan 31, 2024
@Laerte Laerte requested a review from wRAR January 31, 2024 16:16
scrapy/utils/test.py Outdated Show resolved Hide resolved
@wRAR wRAR merged commit 6f73dc0 into scrapy:master Jan 31, 2024
25 of 26 checks passed
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.

Cleanup deprecated fingerprint code in scrapy.utils.request
3 participants