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

Render error when change image size #686

Closed
rafaeltcc opened this issue Nov 30, 2016 · 29 comments · Fixed by #689
Closed

Render error when change image size #686

rafaeltcc opened this issue Nov 30, 2016 · 29 comments · Fixed by #689
Assignees
Labels

Comments

@rafaeltcc
Copy link

rafaeltcc commented Nov 30, 2016

Hi,

I am using a production 4.3.9 Plone with latest collective.cover. If, in my Layout editor, I choose any image size different of default 200:200 for a Basic Tile I get:

There was an error while rendering this tile

I get the following in my log

2016-11-29T23:01:35 ERROR plone.subrequest Error handling subrequest to http://inverta.org/jornal/capa/@@collective.cover.basic/a9379171-0f0a-4981-93c2-4ef4505d54fe
Traceback (most recent call last):
  File "/home/rc/Plone-4.3/buildout-cache/eggs/plone.subrequest-1.6.11-py2.7.egg/plone/subrequest/__init__.py", line 145, in subrequest
    bind=1)
  File "/home/rc/Plone-4.3/buildout-cache/eggs/Zope2-2.13.24-py2.7.egg/ZPublisher/mapply.py", line 78, in mapply
    else: return object(*args)
  File "/home/rc/Plone-4.3/buildout-cache/eggs/plone.tiles-1.5.2-py2.7.egg/plone/tiles/esi.py", line 68, in __call__
    return self.index(*args, **kwargs)
  File "/home/rc/Plone-4.3/buildout-cache/eggs/Zope2-2.13.24-py2.7.egg/Products/Five/browser/pagetemplatefile.py", line 125, in __call__
    return self.im_func(im_self, *args, **kw)
  File "/home/rc/Plone-4.3/buildout-cache/eggs/Zope2-2.13.24-py2.7.egg/Products/Five/browser/pagetemplatefile.py", line 59, in __call__
    sourceAnnotations=getattr(debug_flags, 'sourceAnnotations', 0),
  File "/home/rc/Plone-4.3/buildout-cache/eggs/zope.pagetemplate-3.6.3-py2.7.egg/zope/pagetemplate/pagetemplate.py", line 132, in pt_render
    strictinsert=0, sourceAnnotations=sourceAnnotations
  File "/home/rc/Plone-4.3/buildout-cache/eggs/zope.pagetemplate-3.6.3-py2.7.egg/zope/pagetemplate/pagetemplate.py", line 240, in __call__
    interpreter()
  File "/home/rc/Plone-4.3/buildout-cache/eggs/zope.tal-3.5.2-py2.7.egg/zope/tal/talinterpreter.py", line 271, in __call__
    self.interpret(self.program)
  File "/home/rc/Plone-4.3/buildout-cache/eggs/zope.tal-3.5.2-py2.7.egg/zope/tal/talinterpreter.py", line 343, in interpret
    handlers[opcode](self, args)
  File "/home/rc/Plone-4.3/buildout-cache/eggs/zope.tal-3.5.2-py2.7.egg/zope/tal/talinterpreter.py", line 852, in do_condition
    self.interpret(block)
  File "/home/rc/Plone-4.3/buildout-cache/eggs/zope.tal-3.5.2-py2.7.egg/zope/tal/talinterpreter.py", line 343, in interpret
    handlers[opcode](self, args)
  File "/home/rc/Plone-4.3/buildout-cache/eggs/zope.tal-3.5.2-py2.7.egg/zope/tal/talinterpreter.py", line 821, in do_loop_tal
    self.interpret(block)
  File "/home/rc/Plone-4.3/buildout-cache/eggs/zope.tal-3.5.2-py2.7.egg/zope/tal/talinterpreter.py", line 343, in interpret
    handlers[opcode](self, args)
  File "/home/rc/Plone-4.3/buildout-cache/eggs/zope.tal-3.5.2-py2.7.egg/zope/tal/talinterpreter.py", line 533, in do_optTag_tal
    self.do_optTag(stuff)
  File "/home/rc/Plone-4.3/buildout-cache/eggs/zope.tal-3.5.2-py2.7.egg/zope/tal/talinterpreter.py", line 518, in do_optTag
    return self.no_tag(start, program)
  File "/home/rc/Plone-4.3/buildout-cache/eggs/zope.tal-3.5.2-py2.7.egg/zope/tal/talinterpreter.py", line 513, in no_tag
    self.interpret(program)
  File "/home/rc/Plone-4.3/buildout-cache/eggs/zope.tal-3.5.2-py2.7.egg/zope/tal/talinterpreter.py", line 343, in interpret
    handlers[opcode](self, args)
  File "/home/rc/Plone-4.3/buildout-cache/eggs/zope.tal-3.5.2-py2.7.egg/zope/tal/talinterpreter.py", line 852, in do_condition
    self.interpret(block)
  File "/home/rc/Plone-4.3/buildout-cache/eggs/zope.tal-3.5.2-py2.7.egg/zope/tal/talinterpreter.py", line 343, in interpret
    handlers[opcode](self, args)
  File "/home/rc/Plone-4.3/buildout-cache/eggs/zope.tal-3.5.2-py2.7.egg/zope/tal/talinterpreter.py", line 533, in do_optTag_tal
    self.do_optTag(stuff)
  File "/home/rc/Plone-4.3/buildout-cache/eggs/zope.tal-3.5.2-py2.7.egg/zope/tal/talinterpreter.py", line 518, in do_optTag
    return self.no_tag(start, program)
  File "/home/rc/Plone-4.3/buildout-cache/eggs/zope.tal-3.5.2-py2.7.egg/zope/tal/talinterpreter.py", line 513, in no_tag
    self.interpret(program)
  File "/home/rc/Plone-4.3/buildout-cache/eggs/zope.tal-3.5.2-py2.7.egg/zope/tal/talinterpreter.py", line 343, in interpret
    handlers[opcode](self, args)
  File "/home/rc/Plone-4.3/buildout-cache/eggs/zope.tal-3.5.2-py2.7.egg/zope/tal/talinterpreter.py", line 583, in do_setLocal_tal
    self.engine.setLocal(name, self.engine.evaluateValue(expr))
  File "/home/rc/Plone-4.3/buildout-cache/eggs/zope.tales-3.5.3-py2.7.egg/zope/tales/tales.py", line 696, in evaluate
    return expression(self)
  File "/home/rc/Plone-4.3/buildout-cache/eggs/zope.tales-3.5.3-py2.7.egg/zope/tales/pythonexpr.py", line 59, in __call__
    return eval(self._code, vars)
  File "<string>", line 1, in <module>
  File "/home/rc/Plone-4.3/buildout-cache/eggs/collective.cover-1.3b1-py2.7.egg/collective/cover/browser/scaling.py", line 198, in scale
    **parameters)
  File "/home/rc/Plone-4.3/buildout-cache/eggs/plone.scale-1.4.1-py2.7.egg/plone/scale/storage.py", line 166, in scale
    self._cleanup()
  File "/home/rc/Plone-4.3/buildout-cache/eggs/plone.scale-1.4.1-py2.7.egg/plone/scale/storage.py", line 186, in _cleanup
    value['modified'] < modified_time - KEEP_SCALE_MILLIS):
TypeError: unsupported operand type(s) for -: 'str' and 'int'
rc@inverta:~/Plone-4.3/zeocluster$ tail -f var/client1/event.log
  File "/home/rc/Plone-4.3/buildout-cache/eggs/zope.tales-3.5.3-py2.7.egg/zope/tales/pythonexpr.py", line 59, in __call__
    return eval(self._code, vars)
  File "<string>", line 1, in <module>
  File "/home/rc/Plone-4.3/buildout-cache/eggs/collective.cover-1.3b1-py2.7.egg/collective/cover/browser/scaling.py", line 198, in scale
    **parameters)
  File "/home/rc/Plone-4.3/buildout-cache/eggs/plone.scale-1.4.1-py2.7.egg/plone/scale/storage.py", line 166, in scale
    self._cleanup()
  File "/home/rc/Plone-4.3/buildout-cache/eggs/plone.scale-1.4.1-py2.7.egg/plone/scale/storage.py", line 186, in _cleanup
    value['modified'] < modified_time - KEEP_SCALE_MILLIS):
TypeError: unsupported operand type(s) for -: 'str' and 'int'
@hvelarde
Copy link
Member

I've seen this also lately under Plone 4.3.11 also; seems that we where using plone.scale = 1.3.5 before and now we're using plone.scale = 1.4.1.

we moved the related code from where it was before to a new location on 1.3b1, the previous one can be seen here:

https://github.com/collective/collective.cover/blame/1.2b1/src/collective/cover/tiles/base.py

I think we need to refactor that, because it looks to me overly complex; can you help digging a litle bit on it, @rafaeltcc?

@rafaeltcc
Copy link
Author

rafaeltcc commented Nov 30, 2016 via email

@rodfersou
Copy link
Member

rodfersou commented Nov 30, 2016

apparently, this commit introduced the error plone/plone.scale@8b89545

@hvelarde
Copy link
Member

@rodfersou where do we have to fix it?

@rodfersou
Copy link
Member

@hvelarde still trying to reproduce the problem.. and understand what happen

@hvelarde
Copy link
Member

@datakurre are you dealing with image scaling on tiles in Mosaic? how do you do that? I would love to remove this code from collective.cover at some point.

@datakurre
Copy link
Contributor

@hvelarde Probably pretty much the same way. There's image tile in plone.app.standardtiles and image scaling view for tile in plone.app.tiles. Major difference is that scales are stored inside the same annotation with the tile configuration:

Yet, in Mosaic 2.0 the persistent image tile is deprecated (not enabled by default) in favor of just uploading images as separate content and linking them (and instead of maintaining the complexity of image tiles, the effort should go for better image upload and optional "media repository" - which are still lacking).

@rodfersou
Copy link
Member

rodfersou commented Dec 1, 2016

The problem happen at the code below:

value['modified'] < modified_time - KEEP_SCALE_MILLIS

And the values at my debug session are

ipdb> modified_time
'1480597779.122006'
ipdb> KEEP_SCALE_MILLIS
86400000

The question is.. why modified_time comes as string here.. still looking..

The strange part is that value['modified'] is a string too:

ipdb> value['modified']
'1480597779.122006'

@rodfersou
Copy link
Member

A cast to float in this line should fix the problem.

@hvelarde
Copy link
Member

hvelarde commented Dec 1, 2016

please ask someone in the Plone framework team

@rodfersou
Copy link
Member

@plone/framework-team can someone help giving directions on what is the right way to fix this issue?

Locally my pull request fixed the issue with one cast to float.

@jensens
Copy link
Member

jensens commented Dec 1, 2016

@rodfersou A cast to float sounds fine. Question is, why its a string before.

@rodfersou
Copy link
Member

rodfersou commented Dec 1, 2016

This line explains why.

@hvelarde should I remove the repr and keep the cast conversion to deal with old data?

Or an upgrade step to review all data is better option?

@hvelarde
Copy link
Member

hvelarde commented Dec 1, 2016

@rodfersou please research a little bit more; see: 7f4f4e3

what's the right value type for that attribute? an upgrade step seems a better fix for me.

@rodfersou
Copy link
Member

@jpgimenez do you remember this code?

@rodfersou
Copy link
Member

@hvelarde as your suggestion I investigated and the default Image content type returns DateTime object when we call modified method. Then my steps here would be:

  • Change the code to save DateTime object instead of a string of timespam.
  • Test if this object can be saved into a PersistendDict.
  • Create an upgrade step to fix old tiles persistent data.

@hvelarde
Copy link
Member

hvelarde commented Dec 2, 2016

@rodfersou what about the Dexterity-based version of the Image content type?

@rodfersou
Copy link
Member

the same:

ipdb> from plone import api
ipdb> portal = api.portal.get()
ipdb> images = portal.listFolderContents({'portal_type': 'Image'})
ipdb> images
[<ATImage at /Plone/0Dv0rhwh_1920x1200.jpg>, <Image at /Plone/0f4znrk5_2560x1600.jpg>]
ipdb> images[0].modified()
DateTime('2016/12/01 11:09:28.406506 GMT-2')
ipdb> images[1].modified()
DateTime('2016/12/02 09:26:17.364917 GMT-2')

@rodfersou
Copy link
Member

After implement the proposed changes, the bug still happen when change the image size and save layout.

Still looking what happened.

@hvelarde
Copy link
Member

hvelarde commented Dec 2, 2016

@rodfersou please write a test to demonstrate the issue first.

FYI, works all the way around:

>>> from plone.scale.storage import KEEP_SCALE_MILLIS
>>> from DateTime import DateTime
>>> import time
>>> DateTime(), type(DateTime())
(DateTime('2016/12/02 10:16:41.941549 GMT-2'), <class 'DateTime.DateTime.DateTime'>)
>>> time.time(), type(time.time())
(1480680980.164985, <type 'float'>)
>>> DateTime() < DateTime() - KEEP_SCALE_MILLIS
False
>>> DateTime() > DateTime() - KEEP_SCALE_MILLIS
True
>>> DateTime() < time.time() - KEEP_SCALE_MILLIS
False
>>> DateTime() > time.time() - KEEP_SCALE_MILLIS
True
>>> time.time() < DateTime() - KEEP_SCALE_MILLIS
False
>>> time.time() > DateTime() - KEEP_SCALE_MILLIS
True
>>> time.time() < time.time() - KEEP_SCALE_MILLIS
False
>>> time.time() > time.time() - KEEP_SCALE_MILLIS
True

we better use time.time() as its memory footprint is smaller.

@rodfersou
Copy link
Member

The problem was that my upgrade step was wrong, I need to change the annotation directly to convert the strings into float.

@rodfersou
Copy link
Member

@hvelarde the pull request is ready

@hvelarde
Copy link
Member

hvelarde commented Dec 6, 2016

@rafaeltcc please help us testing the solution provided.

FYI @idgserpro

@rafaeltcc
Copy link
Author

rafaeltcc commented Dec 6, 2016 via email

@hvelarde
Copy link
Member

hvelarde commented Dec 6, 2016

@rafaeltcc yes, and you will need to run an upgrade step in the control panel to fix current tiles.

@rafaeltcc
Copy link
Author

rafaeltcc commented Dec 6, 2016

Hi,

Image resizing in Basic Title is now ok. The changes did it!

Something that was already occurring before and is still failing is that I need to drag and drop an item (I use mostly newsitems) twice so it get added to a Basic Tile. The first time I drag and drop it just doesn't work. This was malfunctioning before I merged the proposed branch. Should I file a new bug?

I noticed some warnings in my log.

2016-12-06 21:12:37 INFO Zope Ready to handle requests
2016-12-06 21:12:47 ERROR PortalTransforms Cannot register transform lynx_dump, using BrokenTransform: Error
Unable to find binary "lynx" in /usr/local/sbin:/usr/local/bin:/usr/bin:/usr/lib/jvm/default/bin:/usr/bin/site_perl:/usr/bin/vendor_perl:/usr/bin/core_perl
2016-12-06 21:13:01 ERROR PortalTransforms Cannot register transform lynx_dump, using BrokenTransform: Error
Unable to find binary "lynx" in /usr/local/sbin:/usr/local/bin:/usr/bin:/usr/lib/jvm/default/bin:/usr/bin/site_perl:/usr/bin/vendor_perl:/usr/bin/core_perl
2016-12-06 21:14:43 INFO collective.cover About to update 1 objects
2016-12-06 21:14:43 INFO collective.cover Tile e1c592338ddd4a588b51c960307cb1f8 at /jornal/capa updated
2016-12-06 21:14:43 INFO collective.cover Tile be4d98b2518a425faa4d15e3a45eb105 at /jornal/capa updated
2016-12-06 21:14:43 INFO collective.cover Tile db5829b1f7fe424eb0659ef920cd9557 at /jornal/capa updated
2016-12-06 21:14:43 INFO collective.cover Tile a000b88a77e640f6ace271abd1b87cc1 at /jornal/capa updated
2016-12-06 21:14:43 INFO collective.cover Tile 713ed403727d4e1bb7c19d9aca89c138 at /jornal/capa updated
2016-12-06 21:14:43 INFO collective.cover Tile 726784a4f19d4f05836a92af64f78f29 at /jornal/capa updated
2016-12-06 21:14:43 INFO collective.cover Done
2016-12-06 21:14:43 INFO GenericSetup Ran upgrade step Fix image field modification time for profile collective.cover:default
2016-12-06 21:14:43 INFO collective.cover JavaScript resources were cooked
2016-12-06 21:14:43 INFO GenericSetup Ran upgrade step Cook JS resources for profile collective.cover:default
2016-12-06 21:14:56 WARNING plone.event The timezone BRT is not a valid timezone from the Olson database or pytz. Falling back to UTC.
2016-12-06 21:14:56 WARNING plone.event The timezone BRT is not a valid timezone from the Olson database or pytz. Falling back to UTC.

@rafaeltcc
Copy link
Author

Something else I just noticed is that the Carrousell Tile just ignores the image size set. The size of the image is adjusted to the space the tile has in the layout.

Also that I need, as in adding content to a Basic Tile to drag an drop an item several time before it eventualy gets loaded.

@rafaeltcc
Copy link
Author

I also noted an empty line in the Carroussell tile configuration. See image link below

github

@hvelarde
Copy link
Member

hvelarde commented Dec 7, 2016

@rafaeltcc to fix the timezone warning you can do something like this:

hvelarde/smdu.portal@1ddcb59

please open new issues for the other stuff so we can take care in a sane way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants