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

Add a warning/error in case of incorrect gcs permissions #4508

Merged

Conversation

michalp2213
Copy link
Contributor

@michalp2213 michalp2213 commented Apr 19, 2020

Added a warning in case of lack of read access to GCS bucket used with FilePipeline and error in case of lack of write access (as it makes it impossible to upload files). Incorrect GCS permissions could be the cause of a problem described in #4346 .

@codecov
Copy link

codecov bot commented Apr 19, 2020

Codecov Report

Merging #4508 into master will decrease coverage by 0.29%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #4508      +/-   ##
==========================================
- Coverage   84.80%   84.50%   -0.30%     
==========================================
  Files         164      164              
  Lines        9880     9903      +23     
  Branches     1464     1482      +18     
==========================================
- Hits         8379     8369      -10     
- Misses       1246     1266      +20     
- Partials      255      268      +13     
Impacted Files Coverage Δ
scrapy/pipelines/files.py 59.93% <0.00%> (-1.74%) ⬇️
scrapy/core/engine.py 87.87% <0.00%> (-5.02%) ⬇️
scrapy/contracts/default.py 84.31% <0.00%> (-3.69%) ⬇️
scrapy/core/downloader/middleware.py 96.36% <0.00%> (-3.64%) ⬇️
scrapy/utils/reactor.py 80.00% <0.00%> (-3.34%) ⬇️
scrapy/http/request/__init__.py 97.26% <0.00%> (-2.74%) ⬇️
scrapy/core/scraper.py 87.97% <0.00%> (-1.20%) ⬇️
scrapy/commands/__init__.py 61.11% <0.00%> (-1.16%) ⬇️
scrapy/crawler.py 88.26% <0.00%> (-1.06%) ⬇️
... and 7 more

)
if 'storage.objects.get' not in permissions:
logger.warning(
"No 'storage.objects.get' permission for GSC bucket %(bucket)s",
Copy link
Member

@kmike kmike Apr 21, 2020

Choose a reason for hiding this comment

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

What do you think about adding more details about this problem - explaining that it would cause the file to be downloaded every time?

Copy link
Contributor Author

@michalp2213 michalp2213 May 4, 2020

Choose a reason for hiding this comment

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

Just pushed a commit. I hope it's fine now.

kmike
kmike approved these changes May 5, 2020
Copy link
Member

@kmike kmike left a comment

Looks good, thanks @michalp2213!

scrapy/pipelines/files.py Outdated Show resolved Hide resolved
…es/files.py

Co-authored-by: Adrián Chaves <adrian@chaves.io>
@Gallaecio Gallaecio merged commit 628c4a5 into scrapy:master May 6, 2020
1 of 2 checks passed
@Gallaecio
Copy link
Member

Gallaecio commented May 6, 2020

Thank you!

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