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

Custom ImagesPipeline doesn't generate thumbnails #2198

Closed
briehanlombaard opened this issue Aug 25, 2016 · 6 comments
Closed

Custom ImagesPipeline doesn't generate thumbnails #2198

briehanlombaard opened this issue Aug 25, 2016 · 6 comments
Labels
Milestone

Comments

@briehanlombaard
Copy link

@briehanlombaard briehanlombaard commented Aug 25, 2016

For some reason, when using a customised ImagesPipeline thumbnails are no longer generated. This seems to be the case for 1.1.1 and 1.1.2 but not for 1.0.0 so somewhere between those releases something must have changed.

This example should reproduce the problem.

@redapple
Copy link
Contributor

@redapple redapple commented Aug 25, 2016

@briehanlombaard , did you mean "but not for 1.1.0" ?

@briehanlombaard
Copy link
Author

@briehanlombaard briehanlombaard commented Aug 25, 2016

@redapple I actually didn't try that version until now, but it does seem to work as well. So the problem must be somewhere between 1.1.0 and 1.1.1

@briehanlombaard
Copy link
Author

@briehanlombaard briehanlombaard commented Aug 26, 2016

I used git bisect to track down the 'bad' commit at c6277c9. I'll see if I can get a fix for the issue if one is needed. I might just be doing something wrong.

@redapple redapple added the bug label Aug 26, 2016
@redapple
Copy link
Contributor

@redapple redapple commented Aug 26, 2016

@briehanlombaard , thank you for continuing the investigation.
I can confirm that thumbnails are not generated with your example starting from scrapy 1.1.1 (worked on 1.1.0)
I'll also have a look in more details.

@redapple
Copy link
Contributor

@redapple redapple commented Aug 26, 2016

@briehanlombaard , so the trouble is indeed with that commit you linked.
Current implementation is looking for ARTICLEIMAGESPIPELINE_IMAGES_THUMBS without looking for IMAGES_THUMBS, contrary to what is done for IMAGES_STORE, and contrary to what the docs currently say.
@pawelmhm , it looks like we still have a backward incompatibility issue with #1891 and the subsequent fix in #1989
@briehanlombaard's use-case looks very reasonable:

  • custom ImagesPipeline but only for converting, no class attribute change
  • use of custom settings for IMAGES_STORE and IMAGES_THUMBS, like the docs recommend

@redapple redapple added this to the v1.2.3 milestone Aug 26, 2016
@redapple redapple added this to the v1.1.3 milestone Aug 26, 2016
@redapple redapple removed this from the v1.2.3 milestone Aug 26, 2016
@djunzu
Copy link
Contributor

@djunzu djunzu commented Sep 9, 2016

@redapple , I am not sure it is a bug. It was coded to work just like that. See this test. I don't remember details, but if I am not wrong it was coded in this way to accommodate class attributes in custom ImagesPipelines.

I think that in order to do this given example work it would be necessary some backward incompatibility with class attributes. And, as far as we know, there is no backward incompatibility with class attributes now.

Maybe the way to go now should be just improve the docs and better explain how to create/use custom pipelines. (And accept this little backward incompatibility as it is.)

@briehanlombaard , you should precede IMAGES_THUMBS with your custom ImagesPipeline class name:

custom_settings = {
    'ITEM_PIPELINES': {
        'spider.ArticleImagesPipeline': 100,
    },
    'IMAGES_STORE': 'articles',
    #'IMAGES_THUMBS': {
    'ARTICLEIMAGESPIPELINE_IMAGES_THUMBS': {
        'small': (290, 150),
        'medium': (610, 320),
    },
    'AUTOTHROTTLE_ENABLED': True,
}

pawelmhm added a commit to pawelmhm/scrapy that referenced this issue Sep 15, 2016
if user has some custom subclass of Image pipeline and no setting for
this pipeline, he should get default settings defined for Image Pipeline.

Fixes scrapy#2198
pawelmhm added a commit to pawelmhm/scrapy that referenced this issue Sep 15, 2016
if user has some custom subclass of Image pipeline and no setting for
this pipeline, he should get default settings defined for Image Pipeline.

Fixes scrapy#2198
redapple added a commit to redapple/scrapy that referenced this issue Sep 19, 2016
if user has some custom subclass of Image pipeline and no setting for
this pipeline, he should get default settings defined for Image Pipeline.

Fixes scrapy#2198
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants