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

Solve incompatibility errors in _compression.py #6261

Merged
merged 3 commits into from
Mar 1, 2024

Conversation

ygalvao
Copy link
Contributor

@ygalvao ygalvao commented Feb 29, 2024

Increase compatibility across different virtual environments, particularly with Conda, when using brotli.Decompressor()

Increase compatibility across different virtual environments, particularly with Conda, when using brotli.Decompressor()
@wRAR
Copy link
Member

wRAR commented Feb 29, 2024

Is this only required when brotlipy is installed instead of/in addition to brotli?

@ygalvao
Copy link
Contributor Author

ygalvao commented Feb 29, 2024

Yes. I don't know why, but when setting up conda environments and installing scrapy and other packages it seems Conda / Mamba only installs brotlipy.

@Gallaecio
Copy link
Member

Maybe we should do something to make Scrapy distinguish the 2, and only enable brotli support when the right one is installed.

@wRAR
Copy link
Member

wRAR commented Feb 29, 2024

Yeah.

@wRAR
Copy link
Member

wRAR commented Feb 29, 2024

when setting up conda environments and installing scrapy and other packages it seems Conda / Mamba only installs brotlipy.

Note that Scrapy currently does not depend on brotli (or on brotlipy), neither on PyPI nor on Conda, and if something installs brotlipy as opposed to not installing anything it's either "other packages" you mentioned or some other things you do when "setting up".

@ygalvao
Copy link
Contributor Author

ygalvao commented Feb 29, 2024

I understand. However, in my current conda env I just have a few packages installed, most of them are well-known ones like Scrapy, Selenium, Pandas, Numpy, etc. and somehow, Conda or Mamba installed brotlipy, which throws an AttributeError when running Scrapy. So this simple change solves this issue, adding compatibility with brotlipy and hence with other packages installed using Conda / Mamba as well.

Copy link

codecov bot commented Mar 1, 2024

Codecov Report

Merging #6261 (9cbdacb) into master (ee51958) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6261   +/-   ##
=======================================
  Coverage   88.89%   88.90%           
=======================================
  Files         161      161           
  Lines       11776    11786   +10     
  Branches     1913     1913           
=======================================
+ Hits        10468    10478   +10     
  Misses        964      964           
  Partials      344      344           
Files Coverage Δ
scrapy/utils/_compression.py 91.89% <100.00%> (+1.26%) ⬆️

@Gallaecio Gallaecio merged commit 4cd94aa into scrapy:master Mar 1, 2024
26 checks passed
Gallaecio pushed a commit that referenced this pull request Mar 1, 2024
@ygalvao ygalvao deleted the ygalvao-patch-1 branch March 1, 2024 17:45
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.

None yet

3 participants